Skip to content

Commit bc944e7

Browse files
authored
Improve messages emitted for exec failures (openai#1659)
1. Emit call_id to exec approval elicitations for mcp client convenience 2. Remove the `-retry` from the call id for the same reason as above but upstream the reset behavior to the mcp client
1 parent 591cb61 commit bc944e7

File tree

5 files changed

+36
-12
lines changed

5 files changed

+36
-12
lines changed

codex-rs/core/src/codex.rs

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,7 @@ impl Session {
253253
pub async fn request_command_approval(
254254
&self,
255255
sub_id: String,
256+
call_id: String,
256257
command: Vec<String>,
257258
cwd: PathBuf,
258259
reason: Option<String>,
@@ -261,6 +262,7 @@ impl Session {
261262
let event = Event {
262263
id: sub_id.clone(),
263264
msg: EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent {
265+
call_id,
264266
command,
265267
cwd,
266268
reason,
@@ -1393,6 +1395,7 @@ async fn handle_container_exec_with_params(
13931395
let rx_approve = sess
13941396
.request_command_approval(
13951397
sub_id.clone(),
1398+
call_id.clone(),
13961399
params.command.clone(),
13971400
params.cwd.clone(),
13981401
None,
@@ -1520,6 +1523,7 @@ async fn handle_sandbox_error(
15201523
let rx_approve = sess
15211524
.request_command_approval(
15221525
sub_id.clone(),
1526+
call_id.clone(),
15231527
params.command.clone(),
15241528
params.cwd.clone(),
15251529
Some("command failed; retry without sandbox?".to_string()),
@@ -1537,9 +1541,7 @@ async fn handle_sandbox_error(
15371541
sess.notify_background_event(&sub_id, "retrying command without sandbox")
15381542
.await;
15391543

1540-
// Emit a fresh Begin event so progress bars reset.
1541-
let retry_call_id = format!("{call_id}-retry");
1542-
sess.notify_exec_command_begin(&sub_id, &retry_call_id, &params)
1544+
sess.notify_exec_command_begin(&sub_id, &call_id, &params)
15431545
.await;
15441546

15451547
// This is an escalated retry; the policy will not be
@@ -1562,14 +1564,8 @@ async fn handle_sandbox_error(
15621564
duration,
15631565
} = retry_output;
15641566

1565-
sess.notify_exec_command_end(
1566-
&sub_id,
1567-
&retry_call_id,
1568-
&stdout,
1569-
&stderr,
1570-
exit_code,
1571-
)
1572-
.await;
1567+
sess.notify_exec_command_end(&sub_id, &call_id, &stdout, &stderr, exit_code)
1568+
.await;
15731569

15741570
let is_success = exit_code == 0;
15751571
let content = format_exec_output(

codex-rs/core/src/protocol.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,8 @@ pub struct ExecCommandEndEvent {
422422

423423
#[derive(Debug, Clone, Deserialize, Serialize)]
424424
pub struct ExecApprovalRequestEvent {
425+
/// Identifier for the associated exec call, if available.
426+
pub call_id: String,
425427
/// The command to be executed.
426428
pub command: Vec<String>,
427429
/// The command's working directory.

codex-rs/mcp-server/src/codex_tool_runner.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ async fn run_codex_tool_session_inner(
156156
EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent {
157157
command,
158158
cwd,
159+
call_id: _,
159160
reason: _,
160161
}) => {
161162
handle_exec_approval_request(

codex-rs/tui/src/chatwidget.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,7 @@ impl ChatWidget<'_> {
314314
self.bottom_pane.set_task_running(false);
315315
}
316316
EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent {
317+
call_id: _,
317318
command,
318319
cwd,
319320
reason,
@@ -362,7 +363,7 @@ impl ChatWidget<'_> {
362363
cwd: _,
363364
}) => {
364365
self.conversation_history
365-
.add_active_exec_command(call_id, command);
366+
.reset_or_add_active_exec_command(call_id, command);
366367
self.request_redraw();
367368
}
368369
EventMsg::PatchApplyBegin(PatchApplyBeginEvent {

codex-rs/tui/src/conversation_history_widget.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,30 @@ impl ConversationHistoryWidget {
235235
self.add_to_history(HistoryCell::new_active_exec_command(call_id, command));
236236
}
237237

238+
/// If an ActiveExecCommand with the same call_id already exists, replace
239+
/// it with a fresh one (resetting start time and view). Otherwise, add a new entry.
240+
pub fn reset_or_add_active_exec_command(&mut self, call_id: String, command: Vec<String>) {
241+
// Find the most recent matching ActiveExecCommand.
242+
let maybe_idx = self.entries.iter().rposition(|entry| {
243+
if let HistoryCell::ActiveExecCommand { call_id: id, .. } = &entry.cell {
244+
id == &call_id
245+
} else {
246+
false
247+
}
248+
});
249+
250+
if let Some(idx) = maybe_idx {
251+
let width = self.cached_width.get();
252+
self.entries[idx].cell = HistoryCell::new_active_exec_command(call_id.clone(), command);
253+
if width > 0 {
254+
let height = self.entries[idx].cell.height(width);
255+
self.entries[idx].line_count.set(height);
256+
}
257+
} else {
258+
self.add_active_exec_command(call_id, command);
259+
}
260+
}
261+
238262
pub fn add_active_mcp_tool_call(
239263
&mut self,
240264
call_id: String,

0 commit comments

Comments
 (0)