openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-18 17:18:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

Track TUI command completion separately from output (#34218)

Browse Source 
## Why

App-server command output deltas populate an exec cell before the command
finishes. Treating the presence of output as completion can therefore mark a
streaming command inactive too early and mishandle it when the turn is
interrupted.

## What changed

- Use the recorded duration, which is set when a command ends, as the exec
  call's completion marker.
- Preserve streamed output when marking unfinished calls as failed and render
  status lines only after completion.
- Keep a single aggregated output representation and render truncated output
  without collecting every line first.
- Append grouped exploration calls in place and coalesce adjacent reads while
  rendering without cloning the calls.

## Testing

- Add snapshots covering active streamed output, transcript output, and the
  resulting history entry after interruption.
- Add coverage for truncating newline-dense output.

GitOrigin-RevId: 420ac1e5fe1b9fcdbdaa5a8998f3e43a5b720254
This commit is contained in:
Charlie Marsh
2026-07-19 19:44:10 +00:00
committed by copyberry
parent 74bfbda9b5
commit 854a82dbfd
10 changed files with 179 additions and 109 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/tui/src/chatwidget/command_lifecycle.rs
+1 -4
View File
@@ -284,7 +284,7 @@ impl ChatWidget {
.active_cell
.as_mut()
.and_then(|c| c.as_any_mut().downcast_mut::<ExecCell>())
&& let Some(new_exec) = cell.with_added_call(
&& cell.add_call(
id.clone(),
command.clone(),
parsed_cmd.clone(),
@@ -292,7 +292,6 @@ impl ChatWidget {
/*interaction_input*/ None,
)
{
*cell = new_exec;
self.bump_active_cell_revision();
} else {
self.flush_active_cell();
@@ -383,13 +382,11 @@ impl ChatWidget {
let output = if is_unified_exec_interaction {
CommandOutput {
exit_code,
formatted_output: String::new(),
aggregated_output: String::new(),
}
} else {
CommandOutput {
exit_code,
formatted_output: aggregated_output.clone(),
aggregated_output,
}
};
codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__live_app_server_command_output_delta_active.snap
+7
View File
@@ -0,0 +1,7 @@
---
source: tui/src/chatwidget/tests/app_server.rs
expression: active
---
• Running printf 'stdout\nstderr\n'
└ stdout
stderr
codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__live_app_server_command_output_delta_interrupted.snap
+8
View File
@@ -0,0 +1,8 @@
---
source: tui/src/chatwidget/tests/app_server.rs
expression: completed
---
$ printf 'stdout\nstderr\n'
stdout
stderr
✗ (1) • <duration>
codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__live_app_server_command_output_delta_transcript.snap
+7
View File
@@ -0,0 +1,7 @@
---
source: tui/src/chatwidget/tests/app_server.rs
expression: lines_to_single_string(&transcript)
---
$ printf 'stdout\nstderr\n'
stdout
stderr
codex-rs/tui/src/chatwidget/tests/app_server.rs
+51
View File
@@ -850,6 +850,57 @@ async fn live_app_server_command_execution_strips_shell_wrapper() {
);
}
#[tokio::test]
async fn live_app_server_command_output_delta_transcript_snapshot() {
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
chat.on_task_started();
begin_exec(&mut chat, "cmd-1", "printf 'stdout\\nstderr\\n'");
for delta in ["stdout\n", "stderr\n"] {
chat.handle_server_notification(
ServerNotification::CommandExecutionOutputDelta(
codex_app_server_protocol::CommandExecutionOutputDeltaNotification {
thread_id: "thread-1".to_string(),
turn_id: "turn-1".to_string(),
item_id: "cmd-1".to_string(),
delta: delta.to_string(),
},
),
/*replay_kind*/ None,
);
}
let active = active_blob(&chat);
assert_chatwidget_snapshot!("live_app_server_command_output_delta_active", active);
let transcript = chat
.active_cell_transcript_lines(/*width*/ 80)
.expect("active exec transcript lines");
assert_chatwidget_snapshot!(
"live_app_server_command_output_delta_transcript",
lines_to_single_string(&transcript)
);
handle_turn_interrupted(&mut chat, "turn-1");
let mut completed = None;
while let Ok(event) = rx.try_recv() {
if let AppEvent::InsertHistoryCell(cell) = event {
let transcript = lines_to_single_string(&cell.transcript_lines(/*width*/ 80));
if transcript.contains("printf 'stdout\\nstderr\\n'") {
completed = Some(transcript);
}
}
}
let completed = completed.expect("expected the interrupted command in history");
let completed = regex_lite::Regex::new(r"(?m) • (?:\d+ms|\d+\.\d+s|\d+m \d+s)$")
.expect("valid duration regex")
.replace(&completed, " • <duration>");
assert_chatwidget_snapshot!(
"live_app_server_command_output_delta_interrupted",
completed
);
}
#[tokio::test]
async fn live_app_server_collab_wait_items_render_history() {
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
codex-rs/tui/src/exec_cell/model.rs
+15 -21
View File
@@ -11,16 +11,14 @@ use std::time::Instant;
use codex_app_server_protocol::CommandExecutionSource as ExecCommandSource;
use codex_protocol::parse_command::ParsedCommand;
#[derive(Clone, Debug, Default)]
#[derive(Debug, Default)]
pub(crate) struct CommandOutput {
pub(crate) exit_code: i32,
/// The aggregated stderr + stdout interleaved.
pub(crate) aggregated_output: String,
/// The formatted output of the command, as seen by the model.
pub(crate) formatted_output: String,
}
#[derive(Debug, Clone)]
#[derive(Debug)]
pub(crate) struct ExecCall {
pub(crate) call_id: String,
pub(crate) command: Vec<String>,
@@ -46,14 +44,14 @@ impl ExecCell {
}
}
pub(crate) fn with_added_call(
&self,
pub(crate) fn add_call(
&mut self,
call_id: String,
command: Vec<String>,
parsed: Vec<ParsedCommand>,
source: ExecCommandSource,
interaction_input: Option<String>,
) -> Option<Self> {
) -> bool {
let call = ExecCall {
call_id,
command,
@@ -65,12 +63,10 @@ impl ExecCell {
interaction_input,
};
if self.is_exploring_cell() && Self::is_exploring_call(&call) {
Some(Self {
calls: [self.calls.clone(), vec![call]].concat(),
animations_enabled: self.animations_enabled,
})
self.calls.push(call);
true
} else {
None
false
}
}
@@ -95,23 +91,21 @@ impl ExecCell {
}
pub(crate) fn should_flush(&self) -> bool {
!self.is_exploring_cell() && self.calls.iter().all(|c| c.output.is_some())
!self.is_exploring_cell() && self.calls.iter().all(|c| c.duration.is_some())
}
pub(crate) fn mark_failed(&mut self) {
for call in self.calls.iter_mut() {
if call.output.is_none() {
if call.duration.is_none() {
let elapsed = call
.start_time
.map(|st| st.elapsed())
.unwrap_or_else(|| Duration::from_millis(0));
call.start_time = None;
call.duration = Some(elapsed);
call.output = Some(CommandOutput {
exit_code: 1,
formatted_output: String::new(),
aggregated_output: String::new(),
});
call.output
.get_or_insert_with(CommandOutput::default)
.exit_code = 1;
}
}
}
@@ -121,13 +115,13 @@ impl ExecCell {
}
pub(crate) fn is_active(&self) -> bool {
self.calls.iter().any(|c| c.output.is_none())
self.calls.iter().any(|c| c.duration.is_none())
}
pub(crate) fn active_start_time(&self) -> Option<Instant> {
self.calls
.iter()
.find(|c| c.output.is_none())
.find(|c| c.duration.is_none())
.and_then(|c| c.start_time)
}
codex-rs/tui/src/exec_cell/render.rs
+68 -54
View File
@@ -128,13 +128,11 @@ pub(crate) fn output_lines(
}
};
let src = aggregated_output;
let lines: Vec<&str> = src.lines().collect();
let total = lines.len();
let total = aggregated_output.lines().count();
let mut out: Vec<Line<'static>> = Vec::new();
let head_end = total.min(line_limit);
for (i, raw) in lines[..head_end].iter().enumerate() {
for (i, raw) in aggregated_output.lines().take(head_end).enumerate() {
let mut line = ansi_escape_line(raw);
let prefix = if !include_prefix {
""
@@ -166,7 +164,7 @@ pub(crate) fn output_lines(
} else {
head_end
};
for raw in lines[tail_start..].iter() {
for raw in aggregated_output.lines().skip(tail_start) {
let mut line = ansi_escape_line(raw);
if include_prefix {
line.spans.insert(0, " ".into());
@@ -221,25 +219,24 @@ impl HistoryCell for ExecCell {
if !call.is_unified_exec_interaction() {
let wrap_width = width.max(1) as usize;
let wrap_opts = RtOptions::new(wrap_width);
for unwrapped in output.formatted_output.lines().map(ansi_escape_line) {
for unwrapped in output.aggregated_output.lines().map(ansi_escape_line) {
let wrapped = adaptive_wrap_line(&unwrapped, wrap_opts.clone());
push_owned_lines(&wrapped, &mut lines);
}
}
let duration = call
.duration
.map(format_duration)
.unwrap_or_else(|| "unknown".to_string());
let mut result: Line = if output.exit_code == 0 {
Line::from("✓".green().bold())
} else {
Line::from(vec![
"✗".red().bold(),
format!(" ({})", output.exit_code).into(),
])
};
result.push_span(format!(" • {duration}").dim());
lines.push(result);
if let Some(duration) = call.duration {
let duration = format_duration(duration);
let mut result: Line = if output.exit_code == 0 {
Line::from("✓".green().bold())
} else {
Line::from(vec![
"✗".red().bold(),
format!(" ({})", output.exit_code).into(),
])
};
result.push_span(format!(" • {duration}").dim());
lines.push(result);
}
}
}
lines
@@ -275,38 +272,32 @@ impl ExecCell {
},
]));
let mut calls = self.calls.clone();
let mut calls = self.calls.as_slice();
let mut out_indented = Vec::new();
while !calls.is_empty() {
let mut call = calls.remove(0);
if call
.parsed
.iter()
.all(|parsed| matches!(parsed, ParsedCommand::Read { .. }))
{
while let Some(next) = calls.first() {
if next
.parsed
.iter()
.all(|parsed| matches!(parsed, ParsedCommand::Read { .. }))
{
call.parsed.extend(next.parsed.clone());
calls.remove(0);
} else {
break;
}
}
}
while let Some((call, remaining)) = calls.split_first() {
let reads_only = call
.parsed
.iter()
.all(|parsed| matches!(parsed, ParsedCommand::Read { .. }));
let group_len = if reads_only {
1 + remaining
.iter()
.take_while(|next| {
next.parsed
.iter()
.all(|parsed| matches!(parsed, ParsedCommand::Read { .. }))
})
.count()
} else {
1
};
let (group, remaining) = calls.split_at(group_len);
calls = remaining;
let call_lines: Vec<(&str, Vec<Span<'static>>)> = if reads_only {
let names = call
.parsed
let names = group
.iter()
.flat_map(|call| &call.parsed)
.map(|parsed| match parsed {
ParsedCommand::Read { name, .. } => name.clone(),
_ => unreachable!(),
@@ -367,7 +358,9 @@ impl ExecCell {
panic!("Expected exactly one call in a command display cell");
};
let layout = EXEC_DISPLAY_LAYOUT;
let success = call.output.as_ref().map(|o| o.exit_code == 0);
let success = call
.duration
.and_then(|_| call.output.as_ref().map(|o| o.exit_code == 0));
let bullet = match success {
Some(true) => "•".green().bold(),
Some(false) => "•".red().bold(),
@@ -736,7 +729,6 @@ mod tests {
let output = CommandOutput {
exit_code: 0,
aggregated_output,
formatted_output: String::new(),
};
let width = 20;
let layout = EXEC_DISPLAY_LAYOUT;
@@ -862,7 +854,6 @@ mod tests {
let output = CommandOutput {
exit_code: 0,
aggregated_output: (1..=7).map(|n| n.to_string()).join("\n"),
formatted_output: String::new(),
};
let rendered: Vec<String> = output_lines(
@@ -879,14 +870,39 @@ mod tests {
.map(render_line_text)
.collect();
assert!(
rendered
.iter()
.any(|line| line.contains("… +3 lines (ctrl + t to view transcript)")),
"expected logical truncation to include transcript hint, got: {rendered:?}"
assert_eq!(
rendered,
vec![
"1",
"2",
"… +3 lines (ctrl + t to view transcript)",
"6",
"7",
]
);
}
#[test]
fn output_lines_handles_newline_dense_output_without_materializing_every_line() {
let output = CommandOutput {
exit_code: 0,
aggregated_output: "\n".repeat(100_000),
};
let rendered = output_lines(
Some(&output),
OutputLinesParams {
line_limit: 5,
only_err: false,
include_angle_pipe: false,
include_prefix: false,
},
);
assert_eq!(rendered.lines.len(), 11);
assert_eq!(rendered.omitted, Some(99_990));
}
#[test]
fn command_truncation_ellipsis_does_not_include_transcript_hint() {
let truncated = ExecCell::limit_lines_from_start(
@@ -1036,7 +1052,6 @@ mod tests {
parsed: Vec::new(),
output: Some(CommandOutput {
exit_code: 0,
formatted_output: String::new(),
aggregated_output: url.to_string(),
}),
source: ExecCommandSource::UserShell,
@@ -1073,7 +1088,6 @@ mod tests {
parsed: Vec::new(),
output: Some(CommandOutput {
exit_code: 0,
formatted_output: url.to_string(),
aggregated_output: url.to_string(),
}),
source: ExecCommandSource::Agent,
codex-rs/tui/src/history_cell/patches.rs
-1
View File
@@ -45,7 +45,6 @@ pub(crate) fn new_patch_apply_failure(stderr: String) -> PlainHistoryCell {
let output = output_lines(
Some(&CommandOutput {
exit_code: 1,
formatted_output: String::new(),
aggregated_output: stderr,
}),
OutputLinesParams {
codex-rs/tui/src/history_cell/tests.rs
+22 -28
View File
@@ -1699,34 +1699,30 @@ fn coalesces_reads_across_multiple_calls() {
// Call 1: Search only
cell.complete_call("c1", CommandOutput::default(), Duration::from_millis(1));
// Call 2: Read A
cell = cell
.with_added_call(
"c2".into(),
vec!["bash".into(), "-lc".into(), "echo".into()],
vec![ParsedCommand::Read {
name: "shimmer.rs".into(),
cmd: "cat shimmer.rs".into(),
path: "shimmer.rs".into(),
}],
ExecCommandSource::Agent,
/*interaction_input*/ None,
)
.unwrap();
assert!(cell.add_call(
"c2".into(),
vec!["bash".into(), "-lc".into(), "echo".into()],
vec![ParsedCommand::Read {
name: "shimmer.rs".into(),
cmd: "cat shimmer.rs".into(),
path: "shimmer.rs".into(),
}],
ExecCommandSource::Agent,
/*interaction_input*/ None,
));
cell.complete_call("c2", CommandOutput::default(), Duration::from_millis(1));
// Call 3: Read B
cell = cell
.with_added_call(
"c3".into(),
vec!["bash".into(), "-lc".into(), "echo".into()],
vec![ParsedCommand::Read {
name: "status_indicator_widget.rs".into(),
cmd: "cat status_indicator_widget.rs".into(),
path: "status_indicator_widget.rs".into(),
}],
ExecCommandSource::Agent,
/*interaction_input*/ None,
)
.unwrap();
assert!(cell.add_call(
"c3".into(),
vec!["bash".into(), "-lc".into(), "echo".into()],
vec![ParsedCommand::Read {
name: "status_indicator_widget.rs".into(),
cmd: "cat status_indicator_widget.rs".into(),
path: "status_indicator_widget.rs".into(),
}],
ExecCommandSource::Agent,
/*interaction_input*/ None,
));
cell.complete_call("c3", CommandOutput::default(), Duration::from_millis(1));
let lines = cell.display_lines(/*width*/ 80);
@@ -1918,7 +1914,6 @@ fn stderr_tail_more_than_five_lines_snapshot() {
&call_id,
CommandOutput {
exit_code: 1,
formatted_output: String::new(),
aggregated_output: stderr,
},
Duration::from_millis(1),
@@ -1966,7 +1961,6 @@ fn ran_cell_multiline_with_stderr_snapshot() {
&call_id,
CommandOutput {
exit_code: 1,
formatted_output: String::new(),
aggregated_output: stderr,
},
Duration::from_millis(5),
codex-rs/tui/src/pager_overlay.rs
-1
View File
@@ -1226,7 +1226,6 @@ mod tests {
CommandOutput {
exit_code: 0,
aggregated_output: "src\nREADME.md\n".into(),
formatted_output: "src\nREADME.md\n".into(),
},
Duration::from_millis(420),
);