From 995f5c36142affadf6f28cb70c040d8a8819df27 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 15 Oct 2025 13:58:40 -0700 Subject: [PATCH] feat: add Vec to ExecApprovalRequestEvent (#5222) This adds `parsed_cmd: Vec` to `ExecApprovalRequestEvent` in the core protocol (`protocol/src/protocol.rs`), which is also what this field is named on `ExecCommandBeginEvent`. Honestly, I don't love the name (it sounds like a single command, but it is actually a list of them), but I don't want to get distracted by a naming discussion right now. This also adds `parsed_cmd` to `ExecCommandApprovalParams` in `codex-rs/app-server-protocol/src/protocol.rs`, so it will be available via `codex app-server`, as well. For consistency, I also updated `ExecApprovalElicitRequestParams` in `codex-rs/mcp-server/src/exec_approval.rs` to include this field under the name `codex_parsed_cmd`, as that struct already has a number of special `codex_*` fields. Note this is the code for when Codex is used as an MCP _server_ and therefore has to conform to the official spec for an MCP elicitation type. --- codex-rs/app-server-protocol/src/protocol.rs | 11 +++++++++++ codex-rs/app-server/src/codex_message_processor.rs | 2 ++ .../tests/suite/codex_message_processor_flow.rs | 4 ++++ codex-rs/core/src/codex.rs | 2 ++ codex-rs/mcp-server/src/codex_tool_runner.rs | 2 ++ codex-rs/mcp-server/src/exec_approval.rs | 4 ++++ codex-rs/mcp-server/tests/suite/codex_tool.rs | 3 +++ codex-rs/protocol/src/protocol.rs | 1 + codex-rs/tui/src/chatwidget/tests.rs | 6 ++++++ codex-rs/tui/tests/fixtures/binary-size-log.jsonl | 8 ++++---- 10 files changed, 39 insertions(+), 4 deletions(-) diff --git a/codex-rs/app-server-protocol/src/protocol.rs b/codex-rs/app-server-protocol/src/protocol.rs index 845a2431..916560c6 100644 --- a/codex-rs/app-server-protocol/src/protocol.rs +++ b/codex-rs/app-server-protocol/src/protocol.rs @@ -9,6 +9,7 @@ use codex_protocol::config_types::ReasoningEffort; use codex_protocol::config_types::ReasoningSummary; use codex_protocol::config_types::SandboxMode; use codex_protocol::config_types::Verbosity; +use codex_protocol::parse_command::ParsedCommand; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::FileChange; @@ -697,6 +698,7 @@ pub struct ExecCommandApprovalParams { pub cwd: PathBuf, #[serde(skip_serializing_if = "Option::is_none")] pub reason: Option, + pub parsed_cmd: Vec, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, TS)] @@ -904,6 +906,9 @@ mod tests { command: vec!["echo".to_string(), "hello".to_string()], cwd: PathBuf::from("/tmp"), reason: Some("because tests".to_string()), + parsed_cmd: vec![ParsedCommand::Unknown { + cmd: "echo hello".to_string(), + }], }; let request = ServerRequest::ExecCommandApproval { request_id: RequestId::Integer(7), @@ -920,6 +925,12 @@ mod tests { "command": ["echo", "hello"], "cwd": "/tmp", "reason": "because tests", + "parsedCmd": [ + { + "type": "unknown", + "cmd": "echo hello" + } + ] } }), serde_json::to_value(&request)?, diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index cb894a51..f455ad32 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -1284,6 +1284,7 @@ async fn apply_bespoke_event_handling( command, cwd, reason, + parsed_cmd, }) => { let params = ExecCommandApprovalParams { conversation_id, @@ -1291,6 +1292,7 @@ async fn apply_bespoke_event_handling( command, cwd, reason, + parsed_cmd, }; let rx = outgoing .send_request(ServerRequestPayload::ExecCommandApproval(params)) diff --git a/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs b/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs index 4dff2a15..27731f84 100644 --- a/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs +++ b/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs @@ -27,6 +27,7 @@ use codex_core::protocol_config_types::ReasoningEffort; use codex_core::protocol_config_types::ReasoningSummary; use codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR; use codex_protocol::config_types::SandboxMode; +use codex_protocol::parse_command::ParsedCommand; use codex_protocol::protocol::Event; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::InputMessageKind; @@ -311,6 +312,9 @@ async fn test_send_user_turn_changes_approval_policy_behavior() { ], cwd: working_directory.clone(), reason: None, + parsed_cmd: vec![ParsedCommand::Unknown { + cmd: "python3 -c 'print(42)'".to_string() + }], }, params ); diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index adeeb476..5e28a817 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -620,6 +620,7 @@ impl Session { warn!("Overwriting existing pending approval for sub_id: {event_id}"); } + let parsed_cmd = parse_command(&command); let event = Event { id: event_id, msg: EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent { @@ -627,6 +628,7 @@ impl Session { command, cwd, reason, + parsed_cmd, }), }; self.send_event(event).await; diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index f09dc98c..370dde0e 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -178,6 +178,7 @@ async fn run_codex_tool_session_inner( cwd, call_id, reason: _, + parsed_cmd, }) => { handle_exec_approval_request( command, @@ -188,6 +189,7 @@ async fn run_codex_tool_session_inner( request_id_str.clone(), event.id.clone(), call_id, + parsed_cmd, ) .await; continue; diff --git a/codex-rs/mcp-server/src/exec_approval.rs b/codex-rs/mcp-server/src/exec_approval.rs index 119481cd..44607b75 100644 --- a/codex-rs/mcp-server/src/exec_approval.rs +++ b/codex-rs/mcp-server/src/exec_approval.rs @@ -4,6 +4,7 @@ use std::sync::Arc; use codex_core::CodexConversation; use codex_core::protocol::Op; use codex_core::protocol::ReviewDecision; +use codex_protocol::parse_command::ParsedCommand; use mcp_types::ElicitRequest; use mcp_types::ElicitRequestParamsRequestedSchema; use mcp_types::JSONRPCErrorError; @@ -35,6 +36,7 @@ pub struct ExecApprovalElicitRequestParams { pub codex_call_id: String, pub codex_command: Vec, pub codex_cwd: PathBuf, + pub codex_parsed_cmd: Vec, } // TODO(mbolin): ExecApprovalResponse does not conform to ElicitResult. See: @@ -56,6 +58,7 @@ pub(crate) async fn handle_exec_approval_request( tool_call_id: String, event_id: String, call_id: String, + codex_parsed_cmd: Vec, ) { let escaped_command = shlex::try_join(command.iter().map(String::as_str)).unwrap_or_else(|_| command.join(" ")); @@ -77,6 +80,7 @@ pub(crate) async fn handle_exec_approval_request( codex_call_id: call_id, codex_command: command, codex_cwd: cwd, + codex_parsed_cmd, }; let params_json = match serde_json::to_value(¶ms) { Ok(value) => value, diff --git a/codex-rs/mcp-server/tests/suite/codex_tool.rs b/codex-rs/mcp-server/tests/suite/codex_tool.rs index 13ce3864..a26892e5 100644 --- a/codex-rs/mcp-server/tests/suite/codex_tool.rs +++ b/codex-rs/mcp-server/tests/suite/codex_tool.rs @@ -3,6 +3,7 @@ use std::env; use std::path::Path; use std::path::PathBuf; +use codex_core::parse_command; use codex_core::protocol::FileChange; use codex_core::protocol::ReviewDecision; use codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR; @@ -176,6 +177,7 @@ fn create_expected_elicitation_request( shlex::try_join(command.iter().map(std::convert::AsRef::as_ref))?, workdir.to_string_lossy() ); + let codex_parsed_cmd = parse_command::parse_command(&command); Ok(JSONRPCRequest { jsonrpc: JSONRPC_VERSION.into(), id: elicitation_request_id, @@ -193,6 +195,7 @@ fn create_expected_elicitation_request( codex_command: command, codex_cwd: workdir.to_path_buf(), codex_call_id: "call1234".to_string(), + codex_parsed_cmd, })?), }) } diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index b7390947..d18c5338 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -1178,6 +1178,7 @@ pub struct ExecApprovalRequestEvent { /// Optional human-readable reason for the approval (e.g. retry without sandbox). #[serde(skip_serializing_if = "Option::is_none")] pub reason: Option, + pub parsed_cmd: Vec, } #[derive(Debug, Clone, Deserialize, Serialize, TS)] diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index a92be433..64130d55 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -392,6 +392,7 @@ fn exec_approval_emits_proposed_command_and_decision_history() { reason: Some( "this is a test reason such as one that would be produced by the model".into(), ), + parsed_cmd: vec![], }; chat.handle_codex_event(Event { id: "sub-short".into(), @@ -433,6 +434,7 @@ fn exec_approval_decision_truncates_multiline_and_long_commands() { reason: Some( "this is a test reason such as one that would be produced by the model".into(), ), + parsed_cmd: vec![], }; chat.handle_codex_event(Event { id: "sub-multi".into(), @@ -480,6 +482,7 @@ fn exec_approval_decision_truncates_multiline_and_long_commands() { command: vec!["bash".into(), "-lc".into(), long], cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), reason: None, + parsed_cmd: vec![], }; chat.handle_codex_event(Event { id: "sub-long".into(), @@ -1317,6 +1320,7 @@ fn approval_modal_exec_snapshot() { reason: Some( "this is a test reason such as one that would be produced by the model".into(), ), + parsed_cmd: vec![], }; chat.handle_codex_event(Event { id: "sub-approve".into(), @@ -1360,6 +1364,7 @@ fn approval_modal_exec_without_reason_snapshot() { command: vec!["bash".into(), "-lc".into(), "echo hello world".into()], cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), reason: None, + parsed_cmd: vec![], }; chat.handle_codex_event(Event { id: "sub-approve-noreason".into(), @@ -1569,6 +1574,7 @@ fn status_widget_and_approval_modal_snapshot() { reason: Some( "this is a test reason such as one that would be produced by the model".into(), ), + parsed_cmd: vec![], }; chat.handle_codex_event(Event { id: "sub-approve-exec".into(), diff --git a/codex-rs/tui/tests/fixtures/binary-size-log.jsonl b/codex-rs/tui/tests/fixtures/binary-size-log.jsonl index 392cf552..2e6bdd40 100644 --- a/codex-rs/tui/tests/fixtures/binary-size-log.jsonl +++ b/codex-rs/tui/tests/fixtures/binary-size-log.jsonl @@ -2492,7 +2492,7 @@ {"ts":"2025-08-09T15:51:59.856Z","dir":"to_tui","kind":"app_event","variant":"RequestRedraw"} {"ts":"2025-08-09T15:51:59.858Z","dir":"to_tui","kind":"app_event","variant":"Redraw"} {"ts":"2025-08-09T15:51:59.939Z","dir":"to_tui","kind":"log_line","line":"[INFO codex_core::codex] FunctionCall: {\"command\":[\"bash\",\"-lc\",\"just fix\"],\"with_escalated_permissions\":true,\"justifica"} -{"ts":"2025-08-09T15:51:59.939Z","dir":"to_tui","kind":"codex_event","payload":{"id":"1","msg":{"type":"exec_approval_request","call_id":"call_KOxVodT3X5ci7LJmudvcovhW","command":["bash","-lc","just fix"],"cwd":"/Users/easong/code/codex/codex-rs","reason":"Run clippy with network and system permissions to apply lint fixes across workspace."}}} +{"ts":"2025-08-09T15:51:59.939Z","dir":"to_tui","kind":"codex_event","payload":{"id":"1","msg":{"type":"exec_approval_request","call_id":"call_KOxVodT3X5ci7LJmudvcovhW","command":["bash","-lc","just fix"],"cwd":"/Users/easong/code/codex/codex-rs","reason":"Run clippy with network and system permissions to apply lint fixes across workspace.","parsed_cmd":[]}}} {"ts":"2025-08-09T15:51:59.939Z","dir":"to_tui","kind":"app_event","variant":"RequestRedraw"} {"ts":"2025-08-09T15:51:59.939Z","dir":"to_tui","kind":"insert_history","lines":5} {"ts":"2025-08-09T15:51:59.939Z","dir":"to_tui","kind":"app_event","variant":"RequestRedraw"} @@ -4172,7 +4172,7 @@ {"ts":"2025-08-09T15:53:09.375Z","dir":"to_tui","kind":"app_event","variant":"RequestRedraw"} {"ts":"2025-08-09T15:53:09.376Z","dir":"to_tui","kind":"app_event","variant":"Redraw"} {"ts":"2025-08-09T15:53:09.448Z","dir":"to_tui","kind":"log_line","line":"[INFO codex_core::codex] FunctionCall: {\"command\":[\"bash\",\"-lc\",\"just fix\"],\"with_escalated_permissions\":true,\"justifica"} -{"ts":"2025-08-09T15:53:09.448Z","dir":"to_tui","kind":"codex_event","payload":{"id":"1","msg":{"type":"exec_approval_request","call_id":"call_POl3hxI2xeszBLv9IOM7L2ir","command":["bash","-lc","just fix"],"cwd":"/Users/easong/code/codex/codex-rs","reason":"Clippy needs broader permissions; allow to run and apply lint fixes."}}} +{"ts":"2025-08-09T15:53:09.448Z","dir":"to_tui","kind":"codex_event","payload":{"id":"1","msg":{"type":"exec_approval_request","call_id":"call_POl3hxI2xeszBLv9IOM7L2ir","command":["bash","-lc","just fix"],"cwd":"/Users/easong/code/codex/codex-rs","reason":"Clippy needs broader permissions; allow to run and apply lint fixes.","parsed_cmd":[]}}} {"ts":"2025-08-09T15:53:09.448Z","dir":"to_tui","kind":"app_event","variant":"RequestRedraw"} {"ts":"2025-08-09T15:53:09.449Z","dir":"to_tui","kind":"insert_history","lines":5} {"ts":"2025-08-09T15:53:09.449Z","dir":"to_tui","kind":"app_event","variant":"RequestRedraw"} @@ -7776,7 +7776,7 @@ {"ts":"2025-08-09T15:58:28.583Z","dir":"to_tui","kind":"app_event","variant":"RequestRedraw"} {"ts":"2025-08-09T15:58:28.590Z","dir":"to_tui","kind":"app_event","variant":"Redraw"} {"ts":"2025-08-09T15:58:28.594Z","dir":"to_tui","kind":"log_line","line":"[INFO codex_core::codex] FunctionCall: {\"command\":[\"bash\",\"-lc\",\"cargo test -p codex-core shell::tests::test_current_she"} -{"ts":"2025-08-09T15:58:28.594Z","dir":"to_tui","kind":"codex_event","payload":{"id":"1","msg":{"type":"exec_approval_request","call_id":"call_iMa8Qnw0dYLba4rVysxebmkV","command":["bash","-lc","cargo test -p codex-core shell::tests::test_current_shell_detects_zsh -- --nocapture"],"cwd":"/Users/easong/code/codex/codex-rs","reason":"Run the macOS shell detection test without sandbox limits so dscl can read user shell."}}} +{"ts":"2025-08-09T15:58:28.594Z","dir":"to_tui","kind":"codex_event","payload":{"id":"1","msg":{"type":"exec_approval_request","call_id":"call_iMa8Qnw0dYLba4rVysxebmkV","command":["bash","-lc","cargo test -p codex-core shell::tests::test_current_shell_detects_zsh -- --nocapture"],"cwd":"/Users/easong/code/codex/codex-rs","reason":"Run the macOS shell detection test without sandbox limits so dscl can read user shell.","parsed_cmd":[]}}} {"ts":"2025-08-09T15:58:28.594Z","dir":"to_tui","kind":"app_event","variant":"RequestRedraw"} {"ts":"2025-08-09T15:58:28.594Z","dir":"to_tui","kind":"insert_history","lines":5} {"ts":"2025-08-09T15:58:28.594Z","dir":"to_tui","kind":"app_event","variant":"RequestRedraw"} @@ -8730,7 +8730,7 @@ {"ts":"2025-08-09T15:59:01.983Z","dir":"to_tui","kind":"app_event","variant":"RequestRedraw"} {"ts":"2025-08-09T15:59:01.985Z","dir":"to_tui","kind":"app_event","variant":"Redraw"} {"ts":"2025-08-09T15:59:02.005Z","dir":"to_tui","kind":"log_line","line":"[INFO codex_core::codex] FunctionCall: {\"command\":[\"bash\",\"-lc\",\"cargo test --all-features\"],\"with_escalated_permissions"} -{"ts":"2025-08-09T15:59:02.005Z","dir":"to_tui","kind":"codex_event","payload":{"id":"1","msg":{"type":"exec_approval_request","call_id":"call_JDFGIuFhYCIiQO1Aq2L9lBO1","command":["bash","-lc","cargo test --all-features"],"cwd":"/Users/easong/code/codex/codex-rs","reason":"Run full test suite without sandbox constraints to validate the merge."}}} +{"ts":"2025-08-09T15:59:02.005Z","dir":"to_tui","kind":"codex_event","payload":{"id":"1","msg":{"type":"exec_approval_request","call_id":"call_JDFGIuFhYCIiQO1Aq2L9lBO1","command":["bash","-lc","cargo test --all-features"],"cwd":"/Users/easong/code/codex/codex-rs","reason":"Run full test suite without sandbox constraints to validate the merge.","parsed_cmd":[]}}} {"ts":"2025-08-09T15:59:02.006Z","dir":"to_tui","kind":"app_event","variant":"RequestRedraw"} {"ts":"2025-08-09T15:59:02.006Z","dir":"to_tui","kind":"insert_history","lines":5} {"ts":"2025-08-09T15:59:02.006Z","dir":"to_tui","kind":"app_event","variant":"RequestRedraw"}