feat: add Vec<ParsedCommand> to ExecApprovalRequestEvent (#5222)

This adds `parsed_cmd: Vec<ParsedCommand>` 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.
This commit is contained in:
Michael Bolin
2025-10-15 13:58:40 -07:00
committed by GitHub
parent 9b53a306e3
commit 995f5c3614
10 changed files with 39 additions and 4 deletions

View File

@@ -9,6 +9,7 @@ use codex_protocol::config_types::ReasoningEffort;
use codex_protocol::config_types::ReasoningSummary; use codex_protocol::config_types::ReasoningSummary;
use codex_protocol::config_types::SandboxMode; use codex_protocol::config_types::SandboxMode;
use codex_protocol::config_types::Verbosity; use codex_protocol::config_types::Verbosity;
use codex_protocol::parse_command::ParsedCommand;
use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::FileChange; use codex_protocol::protocol::FileChange;
@@ -697,6 +698,7 @@ pub struct ExecCommandApprovalParams {
pub cwd: PathBuf, pub cwd: PathBuf,
#[serde(skip_serializing_if = "Option::is_none")] #[serde(skip_serializing_if = "Option::is_none")]
pub reason: Option<String>, pub reason: Option<String>,
pub parsed_cmd: Vec<ParsedCommand>,
} }
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, TS)] #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, TS)]
@@ -904,6 +906,9 @@ mod tests {
command: vec!["echo".to_string(), "hello".to_string()], command: vec!["echo".to_string(), "hello".to_string()],
cwd: PathBuf::from("/tmp"), cwd: PathBuf::from("/tmp"),
reason: Some("because tests".to_string()), reason: Some("because tests".to_string()),
parsed_cmd: vec![ParsedCommand::Unknown {
cmd: "echo hello".to_string(),
}],
}; };
let request = ServerRequest::ExecCommandApproval { let request = ServerRequest::ExecCommandApproval {
request_id: RequestId::Integer(7), request_id: RequestId::Integer(7),
@@ -920,6 +925,12 @@ mod tests {
"command": ["echo", "hello"], "command": ["echo", "hello"],
"cwd": "/tmp", "cwd": "/tmp",
"reason": "because tests", "reason": "because tests",
"parsedCmd": [
{
"type": "unknown",
"cmd": "echo hello"
}
]
} }
}), }),
serde_json::to_value(&request)?, serde_json::to_value(&request)?,

View File

@@ -1284,6 +1284,7 @@ async fn apply_bespoke_event_handling(
command, command,
cwd, cwd,
reason, reason,
parsed_cmd,
}) => { }) => {
let params = ExecCommandApprovalParams { let params = ExecCommandApprovalParams {
conversation_id, conversation_id,
@@ -1291,6 +1292,7 @@ async fn apply_bespoke_event_handling(
command, command,
cwd, cwd,
reason, reason,
parsed_cmd,
}; };
let rx = outgoing let rx = outgoing
.send_request(ServerRequestPayload::ExecCommandApproval(params)) .send_request(ServerRequestPayload::ExecCommandApproval(params))

View File

@@ -27,6 +27,7 @@ use codex_core::protocol_config_types::ReasoningEffort;
use codex_core::protocol_config_types::ReasoningSummary; use codex_core::protocol_config_types::ReasoningSummary;
use codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR; use codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR;
use codex_protocol::config_types::SandboxMode; use codex_protocol::config_types::SandboxMode;
use codex_protocol::parse_command::ParsedCommand;
use codex_protocol::protocol::Event; use codex_protocol::protocol::Event;
use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::InputMessageKind; use codex_protocol::protocol::InputMessageKind;
@@ -311,6 +312,9 @@ async fn test_send_user_turn_changes_approval_policy_behavior() {
], ],
cwd: working_directory.clone(), cwd: working_directory.clone(),
reason: None, reason: None,
parsed_cmd: vec![ParsedCommand::Unknown {
cmd: "python3 -c 'print(42)'".to_string()
}],
}, },
params params
); );

View File

@@ -620,6 +620,7 @@ impl Session {
warn!("Overwriting existing pending approval for sub_id: {event_id}"); warn!("Overwriting existing pending approval for sub_id: {event_id}");
} }
let parsed_cmd = parse_command(&command);
let event = Event { let event = Event {
id: event_id, id: event_id,
msg: EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent { msg: EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent {
@@ -627,6 +628,7 @@ impl Session {
command, command,
cwd, cwd,
reason, reason,
parsed_cmd,
}), }),
}; };
self.send_event(event).await; self.send_event(event).await;

View File

@@ -178,6 +178,7 @@ async fn run_codex_tool_session_inner(
cwd, cwd,
call_id, call_id,
reason: _, reason: _,
parsed_cmd,
}) => { }) => {
handle_exec_approval_request( handle_exec_approval_request(
command, command,
@@ -188,6 +189,7 @@ async fn run_codex_tool_session_inner(
request_id_str.clone(), request_id_str.clone(),
event.id.clone(), event.id.clone(),
call_id, call_id,
parsed_cmd,
) )
.await; .await;
continue; continue;

View File

@@ -4,6 +4,7 @@ use std::sync::Arc;
use codex_core::CodexConversation; use codex_core::CodexConversation;
use codex_core::protocol::Op; use codex_core::protocol::Op;
use codex_core::protocol::ReviewDecision; use codex_core::protocol::ReviewDecision;
use codex_protocol::parse_command::ParsedCommand;
use mcp_types::ElicitRequest; use mcp_types::ElicitRequest;
use mcp_types::ElicitRequestParamsRequestedSchema; use mcp_types::ElicitRequestParamsRequestedSchema;
use mcp_types::JSONRPCErrorError; use mcp_types::JSONRPCErrorError;
@@ -35,6 +36,7 @@ pub struct ExecApprovalElicitRequestParams {
pub codex_call_id: String, pub codex_call_id: String,
pub codex_command: Vec<String>, pub codex_command: Vec<String>,
pub codex_cwd: PathBuf, pub codex_cwd: PathBuf,
pub codex_parsed_cmd: Vec<ParsedCommand>,
} }
// TODO(mbolin): ExecApprovalResponse does not conform to ElicitResult. See: // 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, tool_call_id: String,
event_id: String, event_id: String,
call_id: String, call_id: String,
codex_parsed_cmd: Vec<ParsedCommand>,
) { ) {
let escaped_command = let escaped_command =
shlex::try_join(command.iter().map(String::as_str)).unwrap_or_else(|_| command.join(" ")); 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_call_id: call_id,
codex_command: command, codex_command: command,
codex_cwd: cwd, codex_cwd: cwd,
codex_parsed_cmd,
}; };
let params_json = match serde_json::to_value(&params) { let params_json = match serde_json::to_value(&params) {
Ok(value) => value, Ok(value) => value,

View File

@@ -3,6 +3,7 @@ use std::env;
use std::path::Path; use std::path::Path;
use std::path::PathBuf; use std::path::PathBuf;
use codex_core::parse_command;
use codex_core::protocol::FileChange; use codex_core::protocol::FileChange;
use codex_core::protocol::ReviewDecision; use codex_core::protocol::ReviewDecision;
use codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR; 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))?, shlex::try_join(command.iter().map(std::convert::AsRef::as_ref))?,
workdir.to_string_lossy() workdir.to_string_lossy()
); );
let codex_parsed_cmd = parse_command::parse_command(&command);
Ok(JSONRPCRequest { Ok(JSONRPCRequest {
jsonrpc: JSONRPC_VERSION.into(), jsonrpc: JSONRPC_VERSION.into(),
id: elicitation_request_id, id: elicitation_request_id,
@@ -193,6 +195,7 @@ fn create_expected_elicitation_request(
codex_command: command, codex_command: command,
codex_cwd: workdir.to_path_buf(), codex_cwd: workdir.to_path_buf(),
codex_call_id: "call1234".to_string(), codex_call_id: "call1234".to_string(),
codex_parsed_cmd,
})?), })?),
}) })
} }

View File

@@ -1178,6 +1178,7 @@ pub struct ExecApprovalRequestEvent {
/// Optional human-readable reason for the approval (e.g. retry without sandbox). /// Optional human-readable reason for the approval (e.g. retry without sandbox).
#[serde(skip_serializing_if = "Option::is_none")] #[serde(skip_serializing_if = "Option::is_none")]
pub reason: Option<String>, pub reason: Option<String>,
pub parsed_cmd: Vec<ParsedCommand>,
} }
#[derive(Debug, Clone, Deserialize, Serialize, TS)] #[derive(Debug, Clone, Deserialize, Serialize, TS)]

View File

@@ -392,6 +392,7 @@ fn exec_approval_emits_proposed_command_and_decision_history() {
reason: Some( reason: Some(
"this is a test reason such as one that would be produced by the model".into(), "this is a test reason such as one that would be produced by the model".into(),
), ),
parsed_cmd: vec![],
}; };
chat.handle_codex_event(Event { chat.handle_codex_event(Event {
id: "sub-short".into(), id: "sub-short".into(),
@@ -433,6 +434,7 @@ fn exec_approval_decision_truncates_multiline_and_long_commands() {
reason: Some( reason: Some(
"this is a test reason such as one that would be produced by the model".into(), "this is a test reason such as one that would be produced by the model".into(),
), ),
parsed_cmd: vec![],
}; };
chat.handle_codex_event(Event { chat.handle_codex_event(Event {
id: "sub-multi".into(), id: "sub-multi".into(),
@@ -480,6 +482,7 @@ fn exec_approval_decision_truncates_multiline_and_long_commands() {
command: vec!["bash".into(), "-lc".into(), long], command: vec!["bash".into(), "-lc".into(), long],
cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")),
reason: None, reason: None,
parsed_cmd: vec![],
}; };
chat.handle_codex_event(Event { chat.handle_codex_event(Event {
id: "sub-long".into(), id: "sub-long".into(),
@@ -1317,6 +1320,7 @@ fn approval_modal_exec_snapshot() {
reason: Some( reason: Some(
"this is a test reason such as one that would be produced by the model".into(), "this is a test reason such as one that would be produced by the model".into(),
), ),
parsed_cmd: vec![],
}; };
chat.handle_codex_event(Event { chat.handle_codex_event(Event {
id: "sub-approve".into(), 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()], command: vec!["bash".into(), "-lc".into(), "echo hello world".into()],
cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")),
reason: None, reason: None,
parsed_cmd: vec![],
}; };
chat.handle_codex_event(Event { chat.handle_codex_event(Event {
id: "sub-approve-noreason".into(), id: "sub-approve-noreason".into(),
@@ -1569,6 +1574,7 @@ fn status_widget_and_approval_modal_snapshot() {
reason: Some( reason: Some(
"this is a test reason such as one that would be produced by the model".into(), "this is a test reason such as one that would be produced by the model".into(),
), ),
parsed_cmd: vec![],
}; };
chat.handle_codex_event(Event { chat.handle_codex_event(Event {
id: "sub-approve-exec".into(), id: "sub-approve-exec".into(),

View File

@@ -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.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.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":"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":"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":"insert_history","lines":5}
{"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":"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.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.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":"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.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":"insert_history","lines":5}
{"ts":"2025-08-09T15:53:09.449Z","dir":"to_tui","kind":"app_event","variant":"RequestRedraw"} {"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.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.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":"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":"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":"insert_history","lines":5}
{"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":"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.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: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":"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":"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":"insert_history","lines":5}
{"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":"app_event","variant":"RequestRedraw"}