Add call_id to patch approvals and elicitations (#1660)

Builds on https://github.com/openai/codex/pull/1659 and adds call_id to
a few more places for the same reason.
This commit is contained in:
Gabriel Peal
2025-07-23 12:55:35 -07:00
committed by GitHub
parent bc944e77f5
commit 084236f717
7 changed files with 27 additions and 3 deletions

View File

@@ -279,6 +279,7 @@ impl Session {
pub async fn request_patch_approval( pub async fn request_patch_approval(
&self, &self,
sub_id: String, sub_id: String,
call_id: String,
action: &ApplyPatchAction, action: &ApplyPatchAction,
reason: Option<String>, reason: Option<String>,
grant_root: Option<PathBuf>, grant_root: Option<PathBuf>,
@@ -287,6 +288,7 @@ impl Session {
let event = Event { let event = Event {
id: sub_id.clone(), id: sub_id.clone(),
msg: EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent { msg: EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent {
call_id,
changes: convert_apply_patch_to_protocol(action), changes: convert_apply_patch_to_protocol(action),
reason, reason,
grant_root, grant_root,
@@ -1629,7 +1631,7 @@ async fn apply_patch(
// Compute a readable summary of path changes to include in the // Compute a readable summary of path changes to include in the
// approval request so the user can make an informed decision. // approval request so the user can make an informed decision.
let rx_approve = sess let rx_approve = sess
.request_patch_approval(sub_id.clone(), &action, None, None) .request_patch_approval(sub_id.clone(), call_id.clone(), &action, None, None)
.await; .await;
match rx_approve.await.unwrap_or_default() { match rx_approve.await.unwrap_or_default() {
ReviewDecision::Approved | ReviewDecision::ApprovedForSession => false, ReviewDecision::Approved | ReviewDecision::ApprovedForSession => false,
@@ -1667,7 +1669,13 @@ async fn apply_patch(
)); ));
let rx = sess let rx = sess
.request_patch_approval(sub_id.clone(), &action, reason.clone(), Some(root.clone())) .request_patch_approval(
sub_id.clone(),
call_id.clone(),
&action,
reason.clone(),
Some(root.clone()),
)
.await; .await;
if !matches!( if !matches!(
@@ -1751,6 +1759,7 @@ async fn apply_patch(
let rx = sess let rx = sess
.request_patch_approval( .request_patch_approval(
sub_id.clone(), sub_id.clone(),
call_id.clone(),
&action, &action,
reason.clone(), reason.clone(),
Some(root.clone()), Some(root.clone()),

View File

@@ -435,6 +435,8 @@ pub struct ExecApprovalRequestEvent {
#[derive(Debug, Clone, Deserialize, Serialize)] #[derive(Debug, Clone, Deserialize, Serialize)]
pub struct ApplyPatchApprovalRequestEvent { pub struct ApplyPatchApprovalRequestEvent {
/// Responses API call id for the associated patch apply call, if available.
pub call_id: String,
pub changes: HashMap<PathBuf, FileChange>, pub changes: HashMap<PathBuf, FileChange>,
/// Optional explanatory reason (e.g. request for extra write access). /// Optional explanatory reason (e.g. request for extra write access).
#[serde(skip_serializing_if = "Option::is_none")] #[serde(skip_serializing_if = "Option::is_none")]

View File

@@ -156,7 +156,7 @@ async fn run_codex_tool_session_inner(
EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent { EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent {
command, command,
cwd, cwd,
call_id: _, call_id,
reason: _, reason: _,
}) => { }) => {
handle_exec_approval_request( handle_exec_approval_request(
@@ -167,6 +167,7 @@ async fn run_codex_tool_session_inner(
request_id.clone(), request_id.clone(),
request_id_str.clone(), request_id_str.clone(),
event.id.clone(), event.id.clone(),
call_id,
) )
.await; .await;
continue; continue;
@@ -180,11 +181,13 @@ async fn run_codex_tool_session_inner(
break; break;
} }
EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent { EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent {
call_id,
reason, reason,
grant_root, grant_root,
changes, changes,
}) => { }) => {
handle_patch_approval_request( handle_patch_approval_request(
call_id,
reason, reason,
grant_root, grant_root,
changes, changes,

View File

@@ -32,6 +32,7 @@ pub struct ExecApprovalElicitRequestParams {
pub codex_elicitation: String, pub codex_elicitation: String,
pub codex_mcp_tool_call_id: String, pub codex_mcp_tool_call_id: String,
pub codex_event_id: String, pub codex_event_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,
} }
@@ -45,6 +46,7 @@ pub struct ExecApprovalResponse {
pub decision: ReviewDecision, pub decision: ReviewDecision,
} }
#[allow(clippy::too_many_arguments)]
pub(crate) async fn handle_exec_approval_request( pub(crate) async fn handle_exec_approval_request(
command: Vec<String>, command: Vec<String>,
cwd: PathBuf, cwd: PathBuf,
@@ -53,6 +55,7 @@ pub(crate) async fn handle_exec_approval_request(
request_id: RequestId, request_id: RequestId,
tool_call_id: String, tool_call_id: String,
event_id: String, event_id: String,
call_id: String,
) { ) {
let escaped_command = let escaped_command =
shlex::try_join(command.iter().map(|s| s.as_str())).unwrap_or_else(|_| command.join(" ")); shlex::try_join(command.iter().map(|s| s.as_str())).unwrap_or_else(|_| command.join(" "));
@@ -71,6 +74,7 @@ pub(crate) async fn handle_exec_approval_request(
codex_elicitation: "exec-approval".to_string(), codex_elicitation: "exec-approval".to_string(),
codex_mcp_tool_call_id: tool_call_id.clone(), codex_mcp_tool_call_id: tool_call_id.clone(),
codex_event_id: event_id.clone(), codex_event_id: event_id.clone(),
codex_call_id: call_id,
codex_command: command, codex_command: command,
codex_cwd: cwd, codex_cwd: cwd,
}; };

View File

@@ -27,6 +27,7 @@ pub struct PatchApprovalElicitRequestParams {
pub codex_elicitation: String, pub codex_elicitation: String,
pub codex_mcp_tool_call_id: String, pub codex_mcp_tool_call_id: String,
pub codex_event_id: String, pub codex_event_id: String,
pub codex_call_id: String,
#[serde(skip_serializing_if = "Option::is_none")] #[serde(skip_serializing_if = "Option::is_none")]
pub codex_reason: Option<String>, pub codex_reason: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")] #[serde(skip_serializing_if = "Option::is_none")]
@@ -41,6 +42,7 @@ pub struct PatchApprovalResponse {
#[allow(clippy::too_many_arguments)] #[allow(clippy::too_many_arguments)]
pub(crate) async fn handle_patch_approval_request( pub(crate) async fn handle_patch_approval_request(
call_id: String,
reason: Option<String>, reason: Option<String>,
grant_root: Option<PathBuf>, grant_root: Option<PathBuf>,
changes: HashMap<PathBuf, FileChange>, changes: HashMap<PathBuf, FileChange>,
@@ -66,6 +68,7 @@ pub(crate) async fn handle_patch_approval_request(
codex_elicitation: "patch-approval".to_string(), codex_elicitation: "patch-approval".to_string(),
codex_mcp_tool_call_id: tool_call_id.clone(), codex_mcp_tool_call_id: tool_call_id.clone(),
codex_event_id: event_id.clone(), codex_event_id: event_id.clone(),
codex_call_id: call_id,
codex_reason: reason, codex_reason: reason,
codex_grant_root: grant_root, codex_grant_root: grant_root,
codex_changes: changes, codex_changes: changes,

View File

@@ -171,6 +171,7 @@ fn create_expected_elicitation_request(
codex_event_id, codex_event_id,
codex_command: command, codex_command: command,
codex_cwd: workdir.to_path_buf(), codex_cwd: workdir.to_path_buf(),
codex_call_id: "call1234".to_string(),
})?), })?),
}) })
} }
@@ -384,6 +385,7 @@ fn create_expected_patch_approval_elicitation_request(
codex_reason: reason, codex_reason: reason,
codex_grant_root: grant_root, codex_grant_root: grant_root,
codex_changes: changes, codex_changes: changes,
codex_call_id: "call1234".to_string(),
})?), })?),
}) })
} }

View File

@@ -328,6 +328,7 @@ impl ChatWidget<'_> {
self.bottom_pane.push_approval_request(request); self.bottom_pane.push_approval_request(request);
} }
EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent { EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent {
call_id: _,
changes, changes,
reason, reason,
grant_root, grant_root,