mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
feat: collapse "waiting" of unified_exec (#8257)
Screenshots here but check the snapshot files to see it better <img width="712" height="408" alt="Screenshot 2025-12-18 at 11 58 02" src="https://github.com/user-attachments/assets/84a2c410-0767-4870-84d1-ae1c0d4c445e" /> <img width="523" height="352" alt="Screenshot 2025-12-18 at 11 17 41" src="https://github.com/user-attachments/assets/d029c7ea-0feb-4493-9dca-af43a0c70c52" />
This commit is contained in:
committed by
GitHub
Unverified
parent
3429de21b3
commit
6c76d17713
@@ -558,6 +558,7 @@ impl ChatWidget {
|
||||
fn on_task_complete(&mut self, last_agent_message: Option<String>) {
|
||||
// If a stream is currently active, finalize it.
|
||||
self.flush_answer_stream_with_separator();
|
||||
self.flush_wait_cell();
|
||||
// Mark task stopped and request redraw now that all content is in history.
|
||||
self.bottom_pane.set_task_running(false);
|
||||
self.running_commands.clear();
|
||||
@@ -880,10 +881,54 @@ impl ChatWidget {
|
||||
.iter()
|
||||
.find(|session| session.key == ev.process_id)
|
||||
.map(|session| session.command_display.clone());
|
||||
self.add_to_history(history_cell::new_unified_exec_interaction(
|
||||
command_display,
|
||||
ev.stdin,
|
||||
));
|
||||
if ev.stdin.is_empty() {
|
||||
// Empty stdin means we are still waiting on background output; keep a live shimmer cell.
|
||||
if let Some(wait_cell) = self.active_cell.as_mut().and_then(|cell| {
|
||||
cell.as_any_mut()
|
||||
.downcast_mut::<history_cell::UnifiedExecWaitCell>()
|
||||
}) && wait_cell.matches(command_display.as_deref())
|
||||
{
|
||||
// Same session still waiting; update command display if it shows up late.
|
||||
wait_cell.update_command_display(command_display);
|
||||
self.request_redraw();
|
||||
return;
|
||||
}
|
||||
let has_non_wait_active = matches!(
|
||||
self.active_cell.as_ref(),
|
||||
Some(active)
|
||||
if active
|
||||
.as_any()
|
||||
.downcast_ref::<history_cell::UnifiedExecWaitCell>()
|
||||
.is_none()
|
||||
);
|
||||
if has_non_wait_active {
|
||||
// Do not preempt non-wait active cells with a wait entry.
|
||||
return;
|
||||
}
|
||||
self.flush_wait_cell();
|
||||
self.active_cell = Some(Box::new(history_cell::new_unified_exec_wait_live(
|
||||
command_display,
|
||||
self.config.animations,
|
||||
)));
|
||||
self.request_redraw();
|
||||
} else {
|
||||
if let Some(wait_cell) = self.active_cell.as_ref().and_then(|cell| {
|
||||
cell.as_any()
|
||||
.downcast_ref::<history_cell::UnifiedExecWaitCell>()
|
||||
}) {
|
||||
// Convert the live wait cell into a static "(waited)" entry before logging stdin.
|
||||
let waited_command = wait_cell.command_display().or(command_display.clone());
|
||||
self.active_cell = None;
|
||||
self.add_to_history(history_cell::new_unified_exec_interaction(
|
||||
waited_command,
|
||||
String::new(),
|
||||
));
|
||||
}
|
||||
self.add_to_history(history_cell::new_unified_exec_interaction(
|
||||
command_display,
|
||||
ev.stdin,
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
fn on_patch_apply_begin(&mut self, event: PatchApplyBeginEvent) {
|
||||
@@ -1780,12 +1825,34 @@ impl ChatWidget {
|
||||
}
|
||||
|
||||
fn flush_active_cell(&mut self) {
|
||||
self.flush_wait_cell();
|
||||
if let Some(active) = self.active_cell.take() {
|
||||
self.needs_final_message_separator = true;
|
||||
self.app_event_tx.send(AppEvent::InsertHistoryCell(active));
|
||||
}
|
||||
}
|
||||
|
||||
// Only flush a live wait cell here; other active cells must finalize via their end events.
|
||||
fn flush_wait_cell(&mut self) {
|
||||
// Wait cells are transient: convert them into "(waited)" history entries if present.
|
||||
// Leave non-wait active cells intact so their end events can finalize them.
|
||||
let Some(active) = self.active_cell.take() else {
|
||||
return;
|
||||
};
|
||||
let Some(wait_cell) = active
|
||||
.as_any()
|
||||
.downcast_ref::<history_cell::UnifiedExecWaitCell>()
|
||||
else {
|
||||
self.active_cell = Some(active);
|
||||
return;
|
||||
};
|
||||
self.needs_final_message_separator = true;
|
||||
let cell =
|
||||
history_cell::new_unified_exec_interaction(wait_cell.command_display(), String::new());
|
||||
self.app_event_tx
|
||||
.send(AppEvent::InsertHistoryCell(Box::new(cell)));
|
||||
}
|
||||
|
||||
fn add_to_history(&mut self, cell: impl HistoryCell + 'static) {
|
||||
self.add_boxed_history(Box::new(cell));
|
||||
}
|
||||
|
||||
+9
@@ -0,0 +1,9 @@
|
||||
---
|
||||
source: tui/src/chatwidget/tests.rs
|
||||
expression: combined
|
||||
---
|
||||
↳ Interacted with background terminal · just fix
|
||||
└ (waited)
|
||||
|
||||
↳ Interacted with background terminal · just fix
|
||||
└ ls
|
||||
+8
@@ -0,0 +1,8 @@
|
||||
---
|
||||
source: tui/src/chatwidget/tests.rs
|
||||
expression: active_combined
|
||||
---
|
||||
↳ Interacted with background terminal · just fix
|
||||
└ pwd
|
||||
|
||||
• Waiting for background terminal · just fix
|
||||
+9
@@ -0,0 +1,9 @@
|
||||
---
|
||||
source: tui/src/chatwidget/tests.rs
|
||||
expression: combined
|
||||
---
|
||||
↳ Interacted with background terminal · just fix
|
||||
└ pwd
|
||||
|
||||
↳ Interacted with background terminal · just fix
|
||||
└ (waited)
|
||||
+5
@@ -0,0 +1,5 @@
|
||||
---
|
||||
source: tui/src/chatwidget/tests.rs
|
||||
expression: active_blob(&chat)
|
||||
---
|
||||
• Waiting for background terminal · just fix
|
||||
+6
@@ -0,0 +1,6 @@
|
||||
---
|
||||
source: tui/src/chatwidget/tests.rs
|
||||
expression: combined
|
||||
---
|
||||
↳ Interacted with background terminal · just fix
|
||||
└ (waited)
|
||||
@@ -39,6 +39,7 @@ use codex_core::protocol::ReviewTarget;
|
||||
use codex_core::protocol::StreamErrorEvent;
|
||||
use codex_core::protocol::TaskCompleteEvent;
|
||||
use codex_core::protocol::TaskStartedEvent;
|
||||
use codex_core::protocol::TerminalInteractionEvent;
|
||||
use codex_core::protocol::TokenCountEvent;
|
||||
use codex_core::protocol::TokenUsage;
|
||||
use codex_core::protocol::TokenUsageInfo;
|
||||
@@ -866,6 +867,42 @@ fn begin_exec_with_source(
|
||||
event
|
||||
}
|
||||
|
||||
fn begin_unified_exec_startup(
|
||||
chat: &mut ChatWidget,
|
||||
call_id: &str,
|
||||
process_id: &str,
|
||||
raw_cmd: &str,
|
||||
) -> ExecCommandBeginEvent {
|
||||
let command = vec!["bash".to_string(), "-lc".to_string(), raw_cmd.to_string()];
|
||||
let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from("."));
|
||||
let event = ExecCommandBeginEvent {
|
||||
call_id: call_id.to_string(),
|
||||
process_id: Some(process_id.to_string()),
|
||||
turn_id: "turn-1".to_string(),
|
||||
command,
|
||||
cwd,
|
||||
parsed_cmd: Vec::new(),
|
||||
source: ExecCommandSource::UnifiedExecStartup,
|
||||
interaction_input: None,
|
||||
};
|
||||
chat.handle_codex_event(Event {
|
||||
id: call_id.to_string(),
|
||||
msg: EventMsg::ExecCommandBegin(event.clone()),
|
||||
});
|
||||
event
|
||||
}
|
||||
|
||||
fn terminal_interaction(chat: &mut ChatWidget, call_id: &str, process_id: &str, stdin: &str) {
|
||||
chat.handle_codex_event(Event {
|
||||
id: call_id.to_string(),
|
||||
msg: EventMsg::TerminalInteraction(TerminalInteractionEvent {
|
||||
call_id: call_id.to_string(),
|
||||
process_id: process_id.to_string(),
|
||||
stdin: stdin.to_string(),
|
||||
}),
|
||||
});
|
||||
}
|
||||
|
||||
fn begin_exec(chat: &mut ChatWidget, call_id: &str, raw_cmd: &str) -> ExecCommandBeginEvent {
|
||||
begin_exec_with_source(chat, call_id, raw_cmd, ExecCommandSource::Agent)
|
||||
}
|
||||
@@ -1247,6 +1284,90 @@ async fn unified_exec_end_after_task_complete_is_suppressed() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unified_exec_waiting_multiple_empty_snapshots() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None);
|
||||
begin_unified_exec_startup(&mut chat, "call-wait-1", "proc-1", "just fix");
|
||||
|
||||
terminal_interaction(&mut chat, "call-wait-1a", "proc-1", "");
|
||||
terminal_interaction(&mut chat, "call-wait-1b", "proc-1", "");
|
||||
assert_snapshot!(
|
||||
"unified_exec_waiting_multiple_empty_active",
|
||||
active_blob(&chat)
|
||||
);
|
||||
|
||||
chat.handle_codex_event(Event {
|
||||
id: "turn-wait-1".into(),
|
||||
msg: EventMsg::TaskComplete(TaskCompleteEvent {
|
||||
last_agent_message: None,
|
||||
}),
|
||||
});
|
||||
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
let combined = cells
|
||||
.iter()
|
||||
.map(|lines| lines_to_single_string(lines))
|
||||
.collect::<String>();
|
||||
assert_snapshot!("unified_exec_waiting_multiple_empty_after", combined);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unified_exec_empty_then_non_empty_snapshot() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None);
|
||||
begin_unified_exec_startup(&mut chat, "call-wait-2", "proc-2", "just fix");
|
||||
|
||||
terminal_interaction(&mut chat, "call-wait-2a", "proc-2", "");
|
||||
terminal_interaction(&mut chat, "call-wait-2b", "proc-2", "ls\n");
|
||||
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
let combined = cells
|
||||
.iter()
|
||||
.map(|lines| lines_to_single_string(lines))
|
||||
.collect::<String>();
|
||||
assert_snapshot!("unified_exec_empty_then_non_empty_after", combined);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unified_exec_non_empty_then_empty_snapshots() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None);
|
||||
begin_unified_exec_startup(&mut chat, "call-wait-3", "proc-3", "just fix");
|
||||
|
||||
terminal_interaction(&mut chat, "call-wait-3a", "proc-3", "pwd\n");
|
||||
terminal_interaction(&mut chat, "call-wait-3b", "proc-3", "");
|
||||
let pre_cells = drain_insert_history(&mut rx);
|
||||
let mut active_combined = pre_cells
|
||||
.iter()
|
||||
.map(|lines| lines_to_single_string(lines))
|
||||
.collect::<String>();
|
||||
if !active_combined.is_empty() {
|
||||
active_combined.push('\n');
|
||||
}
|
||||
active_combined.push_str(&active_blob(&chat));
|
||||
assert_snapshot!("unified_exec_non_empty_then_empty_active", active_combined);
|
||||
|
||||
chat.handle_codex_event(Event {
|
||||
id: "turn-wait-3".into(),
|
||||
msg: EventMsg::TaskComplete(TaskCompleteEvent {
|
||||
last_agent_message: None,
|
||||
}),
|
||||
});
|
||||
|
||||
let post_cells = drain_insert_history(&mut rx);
|
||||
let mut combined = pre_cells
|
||||
.iter()
|
||||
.map(|lines| lines_to_single_string(lines))
|
||||
.collect::<String>();
|
||||
let post = post_cells
|
||||
.iter()
|
||||
.map(|lines| lines_to_single_string(lines))
|
||||
.collect::<String>();
|
||||
if !combined.is_empty() && !post.is_empty() {
|
||||
combined.push('\n');
|
||||
}
|
||||
combined.push_str(&post);
|
||||
assert_snapshot!("unified_exec_non_empty_then_empty_after", combined);
|
||||
}
|
||||
|
||||
/// Selecting the custom prompt option from the review popup sends
|
||||
/// OpenReviewCustomPrompt to the app event channel.
|
||||
#[tokio::test]
|
||||
|
||||
@@ -13,6 +13,7 @@ use crate::render::line_utils::line_to_static;
|
||||
use crate::render::line_utils::prefix_lines;
|
||||
use crate::render::line_utils::push_owned_lines;
|
||||
use crate::render::renderable::Renderable;
|
||||
use crate::shimmer::shimmer_spans;
|
||||
use crate::style::user_message_style;
|
||||
use crate::text_formatting::format_and_truncate_tool_result;
|
||||
use crate::text_formatting::truncate_text;
|
||||
@@ -443,6 +444,79 @@ pub(crate) fn new_unified_exec_interaction(
|
||||
UnifiedExecInteractionCell::new(command_display, stdin)
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
// Live-only wait cell that shimmers while we poll; flushes into a static entry later.
|
||||
pub(crate) struct UnifiedExecWaitCell {
|
||||
command_display: Option<String>,
|
||||
animations_enabled: bool,
|
||||
}
|
||||
|
||||
impl UnifiedExecWaitCell {
|
||||
pub(crate) fn new(command_display: Option<String>, animations_enabled: bool) -> Self {
|
||||
Self {
|
||||
command_display: command_display.filter(|display| !display.is_empty()),
|
||||
animations_enabled,
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn matches(&self, command_display: Option<&str>) -> bool {
|
||||
let command_display = command_display.filter(|display| !display.is_empty());
|
||||
match (self.command_display.as_deref(), command_display) {
|
||||
(Some(current), Some(incoming)) => current == incoming,
|
||||
_ => true,
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn update_command_display(&mut self, command_display: Option<String>) {
|
||||
if self.command_display.is_none() {
|
||||
self.command_display = command_display.filter(|display| !display.is_empty());
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn command_display(&self) -> Option<String> {
|
||||
self.command_display.clone()
|
||||
}
|
||||
}
|
||||
|
||||
impl HistoryCell for UnifiedExecWaitCell {
|
||||
fn display_lines(&self, width: u16) -> Vec<Line<'static>> {
|
||||
if width == 0 {
|
||||
return Vec::new();
|
||||
}
|
||||
let wrap_width = width as usize;
|
||||
|
||||
let mut header_spans = vec!["• ".dim()];
|
||||
if self.animations_enabled {
|
||||
header_spans.extend(shimmer_spans("Waiting for background terminal"));
|
||||
} else {
|
||||
header_spans.push("Waiting for background terminal".bold());
|
||||
}
|
||||
if let Some(command) = &self.command_display
|
||||
&& !command.is_empty()
|
||||
{
|
||||
header_spans.push(" · ".dim());
|
||||
header_spans.push(command.clone().dim());
|
||||
}
|
||||
let header = Line::from(header_spans);
|
||||
|
||||
let mut out: Vec<Line<'static>> = Vec::new();
|
||||
let header_wrapped = word_wrap_line(&header, RtOptions::new(wrap_width));
|
||||
push_owned_lines(&header_wrapped, &mut out);
|
||||
out
|
||||
}
|
||||
|
||||
fn desired_height(&self, width: u16) -> u16 {
|
||||
self.display_lines(width).len() as u16
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn new_unified_exec_wait_live(
|
||||
command_display: Option<String>,
|
||||
animations_enabled: bool,
|
||||
) -> UnifiedExecWaitCell {
|
||||
UnifiedExecWaitCell::new(command_display, animations_enabled)
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
struct UnifiedExecSessionsCell {
|
||||
sessions: Vec<String>,
|
||||
@@ -1749,6 +1823,13 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unified_exec_wait_cell_renders_wait() {
|
||||
let cell = new_unified_exec_wait_live(None, false);
|
||||
let lines = render_transcript(&cell);
|
||||
assert_eq!(lines, vec!["• Waiting for background terminal"],);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ps_output_empty_snapshot() {
|
||||
let cell = new_unified_exec_sessions_output(Vec::new());
|
||||
|
||||
Reference in New Issue
Block a user