From b76a562c49b04e85854ec0e9681d2358a80f0527 Mon Sep 17 00:00:00 2001 From: Gabriel Peal Date: Mon, 11 Aug 2025 11:43:58 -0700 Subject: [PATCH] [2/3] Retain the TUI last exec history cell so that it can be updated by the next tool call (#2097) Right now, every time an exec ends, we emit it to history which makes it immutable. In order to be able to update or merge successive tool calls (which will be useful after https://github.com/openai/codex/pull/2095), we need to retain it as the active cell. This also changes the cell to contain the metadata necessary to render it so it can be updated rather than baking in the final text lines when the cell is created. Part 1: https://github.com/openai/codex/pull/2095 Part 3: https://github.com/openai/codex/pull/2110 --- codex-rs/tui/src/chatwidget.rs | 96 +++++++++++++++++++++----------- codex-rs/tui/src/history_cell.rs | 47 +++++++++------- 2 files changed, 89 insertions(+), 54 deletions(-) diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index a3a51aba..1c81b2cf 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -65,7 +65,7 @@ pub(crate) struct ChatWidget<'a> { app_event_tx: AppEventSender, codex_op_tx: UnboundedSender, bottom_pane: BottomPane<'a>, - active_history_cell: Option, + active_exec_cell: Option, config: Config, initial_user_message: Option, total_token_usage: TokenUsage, @@ -114,7 +114,7 @@ fn create_initial_user_message(text: String, image_paths: Vec) -> Optio impl ChatWidget<'_> { fn interrupt_running_task(&mut self) { if self.bottom_pane.is_task_running() { - self.active_history_cell = None; + self.active_exec_cell = None; self.bottom_pane.clear_ctrl_c_quit_hint(); self.submit_op(Op::Interrupt); self.bottom_pane.set_task_running(false); @@ -131,7 +131,7 @@ impl ChatWidget<'_> { fn layout_areas(&self, area: Rect) -> [Rect; 2] { Layout::vertical([ Constraint::Max( - self.active_history_cell + self.active_exec_cell .as_ref() .map_or(0, |c| c.desired_height(area.width)), ), @@ -210,7 +210,7 @@ impl ChatWidget<'_> { has_input_focus: true, enhanced_keys_supported, }), - active_history_cell: None, + active_exec_cell: None, config, initial_user_message: create_initial_user_message( initial_prompt.unwrap_or_default(), @@ -232,7 +232,7 @@ impl ChatWidget<'_> { pub fn desired_height(&self, width: u16) -> u16 { self.bottom_pane.desired_height(width) + self - .active_history_cell + .active_exec_cell .as_ref() .map_or(0, |c| c.desired_height(width)) } @@ -255,6 +255,7 @@ impl ChatWidget<'_> { } fn add_to_history(&mut self, cell: HistoryCell) { + self.flush_active_exec_cell(); self.app_event_tx .send(AppEvent::InsertHistory(cell.plain_lines())); } @@ -298,6 +299,16 @@ impl ChatWidget<'_> { pub(crate) fn handle_codex_event(&mut self, event: Event) { let Event { id, msg } = event; + + match msg { + EventMsg::AgentMessageDelta(_) + | EventMsg::AgentReasoningDelta(_) + | EventMsg::ExecCommandOutputDelta(_) => {} + _ => { + tracing::info!("handle_codex_event: {:?}", msg); + } + } + match msg { EventMsg::SessionConfigured(event) => { self.bottom_pane @@ -456,6 +467,8 @@ impl ChatWidget<'_> { cwd, parsed_cmd, }) => { + // TODO: merge this into the active exec call. + self.flush_active_exec_cell(); self.finalize_active_stream(); // Ensure the status indicator is visible while the command runs. self.bottom_pane @@ -468,9 +481,37 @@ impl ChatWidget<'_> { parsed_cmd: parsed_cmd.clone(), }, ); - self.active_history_cell = + self.active_exec_cell = Some(HistoryCell::new_active_exec_command(command, parsed_cmd)); } + EventMsg::ExecCommandEnd(ExecCommandEndEvent { + call_id, + exit_code, + duration: _, + stdout, + stderr, + }) => { + // Compute summary before moving stdout into the history cell. + let cmd = self.running_commands.remove(&call_id); + let parsed_cmd = match &cmd { + Some(RunningCommand { parsed_cmd, .. }) => parsed_cmd.clone(), + _ => vec![], + }; + if let Some(cmd) = cmd { + // Replace the active running cell with the finalized result, + // but keep it as the active cell so it can be merged with + // subsequent commands before being committed. + self.active_exec_cell = Some(HistoryCell::new_completed_exec_command( + cmd.command, + parsed_cmd, + CommandOutput { + exit_code, + stdout, + stderr, + }, + )); + } + } EventMsg::ExecCommandOutputDelta(_) => { // TODO } @@ -489,36 +530,12 @@ impl ChatWidget<'_> { self.add_to_history(HistoryCell::new_patch_apply_failure(event.stderr)); } } - EventMsg::ExecCommandEnd(ExecCommandEndEvent { - call_id, - exit_code, - duration: _, - stdout, - stderr, - }) => { - // Compute summary before moving stdout into the history cell. - let cmd = self.running_commands.remove(&call_id); - self.active_history_cell = None; - let (command, parsed_cmd) = match cmd { - Some(cmd) => (cmd.command, cmd.parsed_cmd), - None => (vec![call_id], vec![]), - }; - self.add_to_history(HistoryCell::new_completed_exec_command( - command, - parsed_cmd, - CommandOutput { - exit_code, - stdout, - stderr, - }, - )) - } EventMsg::McpToolCallBegin(McpToolCallBeginEvent { call_id: _, invocation, }) => { self.finalize_active_stream(); - self.add_to_history(HistoryCell::new_active_mcp_tool_call(invocation)); + self.active_exec_cell = Some(HistoryCell::new_active_mcp_tool_call(invocation)); } EventMsg::McpToolCallEnd(McpToolCallEndEvent { call_id: _, @@ -526,7 +543,7 @@ impl ChatWidget<'_> { invocation, result, }) => { - self.add_to_history(HistoryCell::new_completed_mcp_tool_call( + let completed = HistoryCell::new_completed_mcp_tool_call( 80, invocation, duration, @@ -535,7 +552,8 @@ impl ChatWidget<'_> { .map(|r| r.is_error.unwrap_or(false)) .unwrap_or(false), result, - )); + ); + self.active_exec_cell = Some(completed); } EventMsg::GetHistoryEntryResponse(event) => { let codex_core::protocol::GetHistoryEntryResponseEvent { @@ -679,11 +697,21 @@ impl ChatWidget<'_> { // Ensure the waiting status is visible (composer replaced). self.bottom_pane .update_status_text("waiting for model".to_string()); + self.flush_active_exec_cell(); self.emit_stream_header(kind); } } + fn flush_active_exec_cell(&mut self) { + if let Some(active) = self.active_exec_cell.take() { + self.app_event_tx + .send(AppEvent::InsertHistory(active.plain_lines())); + } + } + fn stream_push_and_maybe_commit(&mut self, delta: &str) { + self.flush_active_exec_cell(); + self.live_builder.push_fragment(delta); // Commit overflow rows (small batches) while keeping the last N rows visible. @@ -765,7 +793,7 @@ impl WidgetRef for &ChatWidget<'_> { fn render_ref(&self, area: Rect, buf: &mut Buffer) { let [active_cell_area, bottom_pane_area] = self.layout_areas(area); (&self.bottom_pane).render(bottom_pane_area, buf); - if let Some(cell) = &self.active_history_cell { + if let Some(cell) = &self.active_exec_cell { cell.render_ref(active_cell_area, buf); } } diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 3149de35..6b9c5df7 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -81,12 +81,11 @@ pub(crate) enum HistoryCell { /// Message from the user. UserPrompt { view: TextBlock }, - // AgentMessage and AgentReasoning variants were unused and have been removed. - /// An exec tool call that has not finished yet. - ActiveExecCommand { view: TextBlock }, - - /// Completed exec tool call. - CompletedExecCommand { view: TextBlock }, + Exec { + command: Vec, + parsed: Vec, + output: Option, + }, /// An MCP tool call that has not finished yet. ActiveMcpToolCall { view: TextBlock }, @@ -123,7 +122,7 @@ pub(crate) enum HistoryCell { SessionInfo { view: TextBlock }, /// A pending code patch that is awaiting user approval. Mirrors the - /// behaviour of `ActiveExecCommand` so the user sees *what* patch the + /// behaviour of `ExecCell` so the user sees *what* patch the /// model wants to apply before being prompted to approve or deny it. PendingPatch { view: TextBlock }, @@ -173,15 +172,18 @@ impl HistoryCell { | HistoryCell::PromptsOutput { view } | HistoryCell::ErrorEvent { view } | HistoryCell::SessionInfo { view } - | HistoryCell::CompletedExecCommand { view } | HistoryCell::CompletedMcpToolCall { view } | HistoryCell::PendingPatch { view } | HistoryCell::PlanUpdate { view } | HistoryCell::PatchApplyResult { view } - | HistoryCell::ActiveExecCommand { view, .. } | HistoryCell::ActiveMcpToolCall { view, .. } => { view.lines.iter().map(line_to_static).collect() } + HistoryCell::Exec { + command, + parsed, + output, + } => HistoryCell::exec_command_lines(command, parsed, output.as_ref()), HistoryCell::CompletedMcpToolCallWithImageOutput { .. } => vec![ Line::from("tool result (image output omitted)"), Line::from(""), @@ -268,10 +270,7 @@ impl HistoryCell { command: Vec, parsed: Vec, ) -> Self { - let lines = HistoryCell::exec_command_lines(&command, &parsed, None); - HistoryCell::ActiveExecCommand { - view: TextBlock::new(lines), - } + HistoryCell::new_exec_cell(command, parsed, None) } pub(crate) fn new_completed_exec_command( @@ -279,9 +278,18 @@ impl HistoryCell { parsed: Vec, output: CommandOutput, ) -> Self { - let lines = HistoryCell::exec_command_lines(&command, &parsed, Some(&output)); - HistoryCell::CompletedExecCommand { - view: TextBlock::new(lines), + HistoryCell::new_exec_cell(command, parsed, Some(output)) + } + + fn new_exec_cell( + command: Vec, + parsed: Vec, + output: Option, + ) -> Self { + HistoryCell::Exec { + command, + parsed, + output, } } @@ -290,10 +298,9 @@ impl HistoryCell { parsed: &[ParsedCommand], output: Option<&CommandOutput>, ) -> Vec> { - if parsed.is_empty() { - HistoryCell::new_exec_command_generic(command, output) - } else { - HistoryCell::new_parsed_command(parsed, output) + match parsed.is_empty() { + true => HistoryCell::new_exec_command_generic(command, output), + false => HistoryCell::new_parsed_command(parsed, output), } }