mirror of
https://github.com/openai/codex.git
synced 2026-11-05 16:29:31 +00:00
feat: do not close unified exec processes across turns (#10799)
With this PR we do not close the unified exec processes (i.e. background terminals) at the end of a turn unless: * The user interrupt the turn * The user decide to clean the processes through `app-server` or `/clean` I made sure that `codex exec` correctly kill all the processes
This commit is contained in:
1 parent
4e9e6ca243
commit
6cf61725d0
16 files changed
+300
-46
No files matched your search
+1
-2
@@ -5,10 +5,9 @@ expression: "format!(\"{buf:?}\")"
|
||||
Buffer {
|
||||
area: Rect { x: 0, y: 0, width: 50, height: 1 },
|
||||
content: [
|
||||
" 123 background terminals running · /ps to view ",
|
||||
" 123 background terminals running · /ps to view ·",
|
||||
],
|
||||
styles: [
|
||||
x: 0, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
|
||||
x: 48, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
|
||||
]
|
||||
}
|
||||
+1
-2
@@ -5,10 +5,9 @@ expression: "format!(\"{buf:?}\")"
|
||||
Buffer {
|
||||
area: Rect { x: 0, y: 0, width: 50, height: 1 },
|
||||
content: [
|
||||
" 1 background terminal running · /ps to view ",
|
||||
" 1 background terminal running · /ps to view · /c",
|
||||
],
|
||||
styles: [
|
||||
x: 0, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
|
||||
x: 45, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
|
||||
]
|
||||
}
|
||||
@@ -37,7 +37,9 @@ impl UnifiedExecFooter {
|
||||
|
||||
let count = self.processes.len();
|
||||
let plural = if count == 1 { "" } else { "s" };
|
||||
let message = format!(" {count} background terminal{plural} running · /ps to view");
|
||||
let message = format!(
|
||||
" {count} background terminal{plural} running · /ps to view · /clean to close"
|
||||
);
|
||||
let (truncated, _, _) = take_prefix_by_width(&message, width as usize);
|
||||
vec![Line::from(truncated.dim())]
|
||||
}
|
||||
|
||||
@@ -1354,7 +1354,6 @@ impl ChatWidget {
|
||||
self.suppressed_exec_calls.clear();
|
||||
self.last_unified_wait = None;
|
||||
self.unified_exec_wait_streak = None;
|
||||
self.clear_unified_exec_processes();
|
||||
self.request_redraw();
|
||||
|
||||
if !from_replay && self.queued_user_messages.is_empty() {
|
||||
@@ -1575,7 +1574,6 @@ impl ChatWidget {
|
||||
self.suppressed_exec_calls.clear();
|
||||
self.last_unified_wait = None;
|
||||
self.unified_exec_wait_streak = None;
|
||||
self.clear_unified_exec_processes();
|
||||
self.adaptive_chunking.reset();
|
||||
self.stream_controller = None;
|
||||
self.plan_stream_controller = None;
|
||||
@@ -1690,6 +1688,9 @@ impl ChatWidget {
|
||||
fn on_interrupted_turn(&mut self, reason: TurnAbortReason) {
|
||||
// Finalize, log a gentle prompt, and clear running state.
|
||||
self.finalize_turn();
|
||||
if reason == TurnAbortReason::Interrupted {
|
||||
self.clear_unified_exec_processes();
|
||||
}
|
||||
|
||||
if reason != TurnAbortReason::ReviewEnded {
|
||||
self.add_to_history(history_cell::new_error_event(
|
||||
@@ -1819,9 +1820,12 @@ impl ChatWidget {
|
||||
fn on_exec_command_begin(&mut self, ev: ExecCommandBeginEvent) {
|
||||
self.flush_answer_stream_with_separator();
|
||||
if is_unified_exec_source(ev.source) {
|
||||
self.track_unified_exec_process_begin(&ev);
|
||||
if !self.bottom_pane.is_task_running() {
|
||||
return;
|
||||
}
|
||||
// Unified exec may be parsed as Unknown; keep the working indicator visible regardless.
|
||||
self.bottom_pane.ensure_status_indicator();
|
||||
self.track_unified_exec_process_begin(&ev);
|
||||
if !is_standard_tool_call(&ev.parsed_cmd) {
|
||||
return;
|
||||
}
|
||||
@@ -1832,6 +1836,9 @@ impl ChatWidget {
|
||||
|
||||
fn on_exec_command_output_delta(&mut self, ev: ExecCommandOutputDeltaEvent) {
|
||||
self.track_unified_exec_output_chunk(&ev.call_id, &ev.chunk);
|
||||
if !self.bottom_pane.is_task_running() {
|
||||
return;
|
||||
}
|
||||
|
||||
let Some(cell) = self
|
||||
.active_cell
|
||||
@@ -3381,6 +3388,9 @@ impl ChatWidget {
|
||||
SlashCommand::Ps => {
|
||||
self.add_ps_output();
|
||||
}
|
||||
SlashCommand::Clean => {
|
||||
self.clean_background_terminals();
|
||||
}
|
||||
SlashCommand::Mcp => {
|
||||
self.add_mcp_output();
|
||||
}
|
||||
@@ -4467,6 +4477,11 @@ impl ChatWidget {
|
||||
self.add_to_history(history_cell::new_unified_exec_processes_output(processes));
|
||||
}
|
||||
|
||||
fn clean_background_terminals(&mut self) {
|
||||
self.submit_op(Op::CleanBackgroundTerminals);
|
||||
self.add_info_message("Stopping all background terminals.".to_string(), None);
|
||||
}
|
||||
|
||||
fn stop_rate_limit_poller(&mut self) {
|
||||
if let Some(handle) = self.rate_limit_poller.take() {
|
||||
handle.abort();
|
||||
|
||||
+1
-1
@@ -4,7 +4,7 @@ expression: terminal.backend()
|
||||
---
|
||||
" "
|
||||
"• Working (0s • esc to interrupt) "
|
||||
" 1 background terminal running · /ps to view "
|
||||
" 1 background terminal running · /ps to view · /clean to close "
|
||||
" "
|
||||
" "
|
||||
"› Ask Codex to do anything "
|
||||
|
||||
@@ -3216,6 +3216,22 @@ async fn slash_exit_requests_exit() {
|
||||
assert_matches!(rx.try_recv(), Ok(AppEvent::Exit(ExitMode::ShutdownFirst)));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn slash_clean_submits_background_terminal_cleanup() {
|
||||
let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(None).await;
|
||||
|
||||
chat.dispatch_command(SlashCommand::Clean);
|
||||
|
||||
assert_matches!(op_rx.try_recv(), Ok(Op::CleanBackgroundTerminals));
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
assert_eq!(cells.len(), 1, "expected cleanup confirmation message");
|
||||
let rendered = lines_to_single_string(&cells[0]);
|
||||
assert!(
|
||||
rendered.contains("Stopping all background terminals."),
|
||||
"expected cleanup confirmation, got {rendered:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn slash_resume_opens_picker() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await;
|
||||
@@ -4600,6 +4616,42 @@ async fn interrupt_clears_unified_exec_processes() {
|
||||
let _ = drain_insert_history(&mut rx);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn review_ended_keeps_unified_exec_processes() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await;
|
||||
|
||||
begin_unified_exec_startup(&mut chat, "call-1", "process-1", "sleep 5");
|
||||
begin_unified_exec_startup(&mut chat, "call-2", "process-2", "sleep 6");
|
||||
assert_eq!(chat.unified_exec_processes.len(), 2);
|
||||
|
||||
chat.handle_codex_event(Event {
|
||||
id: "turn-1".into(),
|
||||
msg: EventMsg::TurnAborted(codex_core::protocol::TurnAbortedEvent {
|
||||
reason: TurnAbortReason::ReviewEnded,
|
||||
}),
|
||||
});
|
||||
|
||||
assert_eq!(chat.unified_exec_processes.len(), 2);
|
||||
|
||||
chat.add_ps_output();
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
let combined = cells
|
||||
.iter()
|
||||
.map(|lines| lines_to_single_string(lines))
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
assert!(
|
||||
combined.contains("Background terminals"),
|
||||
"expected /ps to remain available after review-ended abort; got {combined:?}"
|
||||
);
|
||||
assert!(
|
||||
combined.contains("sleep 5") && combined.contains("sleep 6"),
|
||||
"expected /ps to list running unified exec processes; got {combined:?}"
|
||||
);
|
||||
|
||||
let _ = drain_insert_history(&mut rx);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn interrupt_clears_unified_exec_wait_streak_snapshot() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await;
|
||||
@@ -4634,7 +4686,7 @@ async fn interrupt_clears_unified_exec_wait_streak_snapshot() {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn turn_complete_clears_unified_exec_processes() {
|
||||
async fn turn_complete_keeps_unified_exec_processes() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await;
|
||||
|
||||
begin_unified_exec_startup(&mut chat, "call-1", "process-1", "sleep 5");
|
||||
@@ -4648,7 +4700,23 @@ async fn turn_complete_clears_unified_exec_processes() {
|
||||
}),
|
||||
});
|
||||
|
||||
assert!(chat.unified_exec_processes.is_empty());
|
||||
assert_eq!(chat.unified_exec_processes.len(), 2);
|
||||
|
||||
chat.add_ps_output();
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
let combined = cells
|
||||
.iter()
|
||||
.map(|lines| lines_to_single_string(lines))
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
assert!(
|
||||
combined.contains("Background terminals"),
|
||||
"expected /ps to remain available after turn complete; got {combined:?}"
|
||||
);
|
||||
assert!(
|
||||
combined.contains("sleep 5") && combined.contains("sleep 6"),
|
||||
"expected /ps to list running unified exec processes; got {combined:?}"
|
||||
);
|
||||
|
||||
let _ = drain_insert_history(&mut rx);
|
||||
}
|
||||
|
||||
@@ -43,6 +43,7 @@ pub enum SlashCommand {
|
||||
Feedback,
|
||||
Rollout,
|
||||
Ps,
|
||||
Clean,
|
||||
Personality,
|
||||
TestApproval,
|
||||
}
|
||||
@@ -68,6 +69,7 @@ impl SlashCommand {
|
||||
SlashCommand::DebugConfig => "show config layers and requirement sources for debugging",
|
||||
SlashCommand::Statusline => "configure which items appear in the status line",
|
||||
SlashCommand::Ps => "list background terminals",
|
||||
SlashCommand::Clean => "stop all background terminals",
|
||||
SlashCommand::Model => "choose what model and reasoning effort to use",
|
||||
SlashCommand::Personality => "choose a communication style for Codex",
|
||||
SlashCommand::Plan => "switch to Plan mode",
|
||||
@@ -124,6 +126,7 @@ impl SlashCommand {
|
||||
| SlashCommand::Status
|
||||
| SlashCommand::DebugConfig
|
||||
| SlashCommand::Ps
|
||||
| SlashCommand::Clean
|
||||
| SlashCommand::Mcp
|
||||
| SlashCommand::Apps
|
||||
| SlashCommand::Feedback
|
||||
|
||||
Reference in new issue
Block a user