Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion codex-rs/app-server/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,7 @@ You can cancel a running Turn with `turn/interrupt`.
{ "id": 31, "result": {} }
```

The server requests cancellations for running subprocesses, then emits a `turn/completed` event with `status: "interrupted"`. Rely on the `turn/completed` to know when Codex-side cleanup is done.
The server requests cancellation of the active turn, then emits a `turn/completed` event with `status: "interrupted"`. This does not terminate background terminals; use `thread/backgroundTerminals/clean` when you explicitly want to stop those shells. Rely on the `turn/completed` event to know when turn interruption has finished.

### Example: Clean background terminals

Expand Down
4 changes: 1 addition & 3 deletions codex-rs/core/src/tasks/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ pub(crate) use user_shell::UserShellCommandTask;
pub(crate) use user_shell::execute_user_shell_command;

const GRACEFULL_INTERRUPTION_TIMEOUT_MS: u64 = 100;
const TURN_ABORTED_INTERRUPTED_GUIDANCE: &str = "The user interrupted the previous turn on purpose. Any running unified exec processes were terminated. If any tools/commands were aborted, they may have partially executed; verify current state before retrying.";
const TURN_ABORTED_INTERRUPTED_GUIDANCE: &str = "The user interrupted the previous turn on purpose. Any running unified exec processes may still be running in the background. If any tools/commands were aborted, they may have partially executed; verify current state before retrying.";

fn emit_turn_network_proxy_metric(
session_telemetry: &SessionTelemetry,
Expand Down Expand Up @@ -394,8 +394,6 @@ impl Session {
}

pub(crate) async fn cleanup_after_interrupt(&self, turn_context: &Arc<TurnContext>) {
self.close_unified_exec_processes().await;

if let Some(manager) = turn_context.js_repl.manager_if_initialized()
&& let Err(err) = manager.interrupt_turn_exec(&turn_context.sub_id).await
{
Expand Down
72 changes: 41 additions & 31 deletions codex-rs/core/src/unified_exec/process_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,9 +191,29 @@ impl UnifiedExecProcessManager {
emitter.emit(event_ctx, ToolEventStage::Begin).await;

start_streaming_output(&process, context, Arc::clone(&transcript));
let yield_time_ms = clamp_yield_time(request.yield_time_ms);

let start = Instant::now();
// Persist live sessions before the initial yield wait so interrupting the
// turn cannot drop the last Arc and terminate the background process.
let process_started_alive = !process.has_exited() && process.exit_code().is_none();
if process_started_alive {
let network_approval_id = deferred_network_approval
.as_ref()
.map(|deferred| deferred.registration_id().to_string());
self.store_process(
Arc::clone(&process),
context,
&request.command,
cwd.clone(),
start,
request.process_id,
request.tty,
network_approval_id,
Arc::clone(&transcript),
)
.await;
}

let yield_time_ms = clamp_yield_time(request.yield_time_ms);
// For the initial exec_command call, we both stream output to events
// (via start_streaming_output above) and collect a snapshot here for
// the tool response body.
Expand Down Expand Up @@ -222,15 +242,28 @@ impl UnifiedExecProcessManager {
let wall_time = Instant::now().saturating_duration_since(start);

let text = String::from_utf8_lossy(&collected).to_string();
let exit_code = process.exit_code();
let has_exited = process.has_exited() || exit_code.is_some();
let chunk_id = generate_chunk_id();
let process_id = request.process_id;

if has_exited {
let (response_process_id, exit_code) = if process_started_alive {
match self.refresh_process_state(process_id).await {
ProcessStatus::Alive {
exit_code,
process_id,
..
} => (Some(process_id), exit_code),
ProcessStatus::Exited { exit_code, .. } => {
process.check_for_sandbox_denial_with_text(&text).await?;
(None, exit_code)
}
ProcessStatus::Unknown => {
return Err(UnifiedExecError::UnknownProcessId { process_id });
}
}
} else {
// Short‑lived command: emit ExecCommandEnd immediately using the
// same helper as the background watcher, so all end events share
// one implementation.
let exit_code = process.exit_code();
let exit = exit_code.unwrap_or(-1);
emit_exec_end_for_unified_exec(
Arc::clone(&context.session),
Expand All @@ -253,26 +286,7 @@ impl UnifiedExecProcessManager {
)
.await;
process.check_for_sandbox_denial_with_text(&text).await?;
} else {
// Long‑lived command: persist the process so write_stdin can reuse
// it, and register a background watcher that will emit
// ExecCommandEnd when the PTY eventually exits (even if no further
// tool calls are made).
let network_approval_id = deferred_network_approval
.as_ref()
.map(|deferred| deferred.registration_id().to_string());
self.store_process(
Arc::clone(&process),
context,
&request.command,
cwd.clone(),
start,
process_id,
request.tty,
network_approval_id,
Arc::clone(&transcript),
)
.await;
(None, exit_code)
};

let original_token_count = approx_token_count(&text);
Expand All @@ -282,11 +296,7 @@ impl UnifiedExecProcessManager {
wall_time,
raw_output: collected,
max_output_tokens: request.max_output_tokens,
process_id: if has_exited {
None
} else {
Some(request.process_id)
},
process_id: response_process_id,
exit_code,
original_token_count: Some(original_token_count),
session_command: Some(request.command.clone()),
Expand Down
9 changes: 8 additions & 1 deletion codex-rs/core/tests/suite/unified_exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2018,7 +2018,7 @@ async fn unified_exec_keeps_long_running_session_after_turn_end() -> Result<()>
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn unified_exec_interrupt_terminates_long_running_session() -> Result<()> {
async fn unified_exec_interrupt_preserves_long_running_session() -> Result<()> {
skip_if_no_network!(Ok(()));
skip_if_sandbox!(Ok(()));
skip_if_windows!(Ok(()));
Expand Down Expand Up @@ -2092,6 +2092,13 @@ async fn unified_exec_interrupt_terminates_long_running_session() -> Result<()>

codex.submit(Op::Interrupt).await?;
wait_for_event(&codex, |event| matches!(event, EventMsg::TurnAborted(_))).await;

assert!(
process_is_alive(&pid)?,
"expected unified exec process to remain alive after interrupt"
);

codex.submit(Op::CleanBackgroundTerminals).await?;
wait_for_process_exit(&pid).await?;

Ok(())
Expand Down
3 changes: 2 additions & 1 deletion codex-rs/protocol/src/protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,11 +193,12 @@ pub struct ConversationTextParams {
#[allow(clippy::large_enum_variant)]
#[non_exhaustive]
pub enum Op {
/// Abort current task.
/// Abort current task without terminating background terminal processes.
/// This server sends [`EventMsg::TurnAborted`] in response.
Interrupt,

/// Terminate all running background terminal processes for this thread.
/// Use this when callers intentionally want to stop long-lived background shells.
CleanBackgroundTerminals,

/// Start a realtime conversation stream.
Expand Down
23 changes: 21 additions & 2 deletions codex-rs/tui/src/bottom_pane/slash_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
//! The same sandbox- and feature-gating rules are used by both the composer
//! and the command popup. Centralizing them here keeps those call sites small
//! and ensures they stay in sync.
use std::str::FromStr;

use codex_utils_fuzzy_match::fuzzy_match;

use crate::slash_command::SlashCommand;
Expand Down Expand Up @@ -38,10 +40,11 @@ pub(crate) fn builtins_for_input(flags: BuiltinCommandFlags) -> Vec<(&'static st

/// Find a single built-in command by exact name, after applying the gating rules.
pub(crate) fn find_builtin_command(name: &str, flags: BuiltinCommandFlags) -> Option<SlashCommand> {
let cmd = SlashCommand::from_str(name).ok()?;
builtins_for_input(flags)
.into_iter()
.find(|(command_name, _)| *command_name == name)
.map(|(_, cmd)| cmd)
.any(|(_, visible_cmd)| visible_cmd == cmd)
.then_some(cmd)
}

/// Whether any visible built-in fuzzily matches the provided prefix.
Expand Down Expand Up @@ -82,6 +85,22 @@ mod tests {
);
}

#[test]
fn stop_command_resolves_for_dispatch() {
assert_eq!(
find_builtin_command("stop", all_enabled_flags()),
Some(SlashCommand::Stop)
);
}

#[test]
fn clean_command_alias_resolves_for_dispatch() {
assert_eq!(
find_builtin_command("clean", all_enabled_flags()),
Some(SlashCommand::Stop)
);
}

#[test]
fn fast_command_is_hidden_when_disabled() {
let mut flags = all_enabled_flags();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ expression: "format!(\"{buf:?}\")"
Buffer {
area: Rect { x: 0, y: 0, width: 50, height: 1 },
content: [
" 1 background terminal running · /ps to view · /c",
" 1 background terminal running · /ps to view · /s",
],
styles: [
x: 0, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/tui/src/bottom_pane/unified_exec_footer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ impl UnifiedExecFooter {
let count = self.processes.len();
let plural = if count == 1 { "" } else { "s" };
Some(format!(
"{count} background terminal{plural} running · /ps to view · /clean to close"
"{count} background terminal{plural} running · /ps to view · /stop to close"
))
}

Expand Down
17 changes: 3 additions & 14 deletions codex-rs/tui/src/chatwidget.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2172,9 +2172,6 @@ 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();
}
let send_pending_steers_immediately = self.submit_pending_steers_after_interrupt;
self.submit_pending_steers_after_interrupt = false;
if reason != TurnAbortReason::ReviewEnded {
Expand Down Expand Up @@ -2834,14 +2831,6 @@ impl ChatWidget {
}
}

fn clear_unified_exec_processes(&mut self) {
if self.unified_exec_processes.is_empty() {
return;
}
self.unified_exec_processes.clear();
self.sync_unified_exec_footer();
}

fn on_mcp_tool_call_begin(&mut self, ev: McpToolCallBeginEvent) {
let ev2 = ev.clone();
self.defer_or_handle(|q| q.push_mcp_begin(ev), |s| s.handle_mcp_begin_now(ev2));
Expand Down Expand Up @@ -4282,7 +4271,7 @@ impl ChatWidget {
}

pub(crate) fn can_run_ctrl_l_clear_now(&mut self) -> bool {
// Ctrl+L is not a slash command, but it follows /clear's current rule:
// Ctrl+L is not a slash command, but it follows /clear's rule:
// block while a task is running.
if !self.bottom_pane.is_task_running() {
return true;
Expand Down Expand Up @@ -4557,7 +4546,7 @@ impl ChatWidget {
SlashCommand::Ps => {
self.add_ps_output();
}
SlashCommand::Clean => {
SlashCommand::Stop => {
self.clean_background_terminals();
}
SlashCommand::MemoryDrop => {
Expand Down Expand Up @@ -8601,7 +8590,7 @@ impl ChatWidget {
///
/// The first press arms a time-bounded quit shortcut and shows a footer hint via the bottom
/// pane. If cancellable work is active, Ctrl+C also submits `Op::Interrupt` after the shortcut
/// is armed.
/// is armed; this interrupts the turn but intentionally preserves background terminals.
///
/// If the same quit shortcut is pressed again before expiry, this requests a shutdown-first
/// quit.
Expand Down
18 changes: 11 additions & 7 deletions codex-rs/tui/src/chatwidget/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3820,7 +3820,7 @@ async fn enqueueing_history_prompt_multiple_times_is_stable() {
}

#[tokio::test]
async fn streaming_final_answer_keeps_task_running_state() {
async fn streaming_final_answer_ctrl_c_interrupt_preserves_background_shells() {
let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(None).await;
chat.thread_id = Some(ThreadId::new());

Expand All @@ -3843,12 +3843,16 @@ async fn streaming_final_answer_keeps_task_running_state() {
);
assert_matches!(op_rx.try_recv(), Err(TryRecvError::Empty));

begin_unified_exec_startup(&mut chat, "call-1", "process-1", "npm run dev");
assert_eq!(chat.unified_exec_processes.len(), 1);

chat.handle_key_event(KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL));
match op_rx.try_recv() {
Ok(Op::Interrupt) => {}
other => panic!("expected Op::Interrupt, got {other:?}"),
}
assert!(!chat.bottom_pane.quit_shortcut_hint_visible());
assert_eq!(chat.unified_exec_processes.len(), 1);
}

#[tokio::test]
Expand Down Expand Up @@ -6025,10 +6029,10 @@ async fn slash_exit_requests_exit() {
}

#[tokio::test]
async fn slash_clean_submits_background_terminal_cleanup() {
async fn slash_stop_submits_background_terminal_cleanup() {
let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(None).await;

chat.dispatch_command(SlashCommand::Clean);
chat.dispatch_command(SlashCommand::Stop);

assert_matches!(op_rx.try_recv(), Ok(Op::CleanBackgroundTerminals));
let cells = drain_insert_history(&mut rx);
Expand Down Expand Up @@ -9061,7 +9065,7 @@ async fn interrupt_prepends_queued_messages_before_existing_composer_text() {
}

#[tokio::test]
async fn interrupt_clears_unified_exec_processes() {
async fn interrupt_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");
Expand All @@ -9076,7 +9080,7 @@ async fn interrupt_clears_unified_exec_processes() {
}),
});

assert!(chat.unified_exec_processes.is_empty());
assert_eq!(chat.unified_exec_processes.len(), 2);

let _ = drain_insert_history(&mut rx);
}
Expand Down Expand Up @@ -9119,7 +9123,7 @@ async fn review_ended_keeps_unified_exec_processes() {
}

#[tokio::test]
async fn interrupt_clears_unified_exec_wait_streak_snapshot() {
async fn interrupt_preserves_unified_exec_wait_streak_snapshot() {
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await;

chat.handle_codex_event(Event {
Expand Down Expand Up @@ -9150,7 +9154,7 @@ async fn interrupt_clears_unified_exec_wait_streak_snapshot() {
.collect::<Vec<_>>()
.join("\n");
let snapshot = format!("cells={}\n{combined}", cells.len());
assert_snapshot!("interrupt_clears_unified_exec_wait_streak", snapshot);
assert_snapshot!("interrupt_preserves_unified_exec_wait_streak", snapshot);
}

#[tokio::test]
Expand Down
Loading
Loading