mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
Keep the TUI open when starting a turn fails (#34636)
## What changed - Handle app-server `turn/start` rejections for pending user turns as turn errors instead of exiting the TUI. - Finalize the pending turn, display the failure in the transcript, and resume queued input handling. - Add coverage that submits a turn to a missing thread and verifies that the error is shown while the app continues running. GitOrigin-RevId: 6d291ba62b9e50d49dd4e3d7c429e98c1f754680
This commit is contained in:
@@ -413,7 +413,8 @@ impl App {
|
||||
return Ok(AppRunControl::Exit(ExitReason::Fatal(message)));
|
||||
}
|
||||
AppEvent::CodexOp(op) => {
|
||||
if matches!(&op, AppCommand::UserTurn { .. }) {
|
||||
let is_user_turn = matches!(&op, AppCommand::UserTurn { .. });
|
||||
if is_user_turn {
|
||||
self.handle_draw_pre_render(tui)?;
|
||||
if self.transcript_reflow.has_pending_reflow() {
|
||||
self.transcript_reflow.schedule_immediate();
|
||||
@@ -423,7 +424,21 @@ impl App {
|
||||
self.render_chat_widget_frame(tui)?;
|
||||
}
|
||||
self.chat_widget.prepare_local_op_submission(&op);
|
||||
self.submit_active_thread_op(app_server, op).await?;
|
||||
if let Err(err) = self.submit_active_thread_op(app_server, op).await {
|
||||
let handled = is_user_turn
|
||||
&& matches!(
|
||||
err.downcast_ref::<TypedRequestError>(),
|
||||
Some(TypedRequestError::Server { method, .. })
|
||||
if method == "turn/start"
|
||||
)
|
||||
&& self
|
||||
.chat_widget
|
||||
.handle_turn_start_rejection(format!("Failed to start turn: {err:#}"));
|
||||
if !handled {
|
||||
return Err(err);
|
||||
}
|
||||
tracing::error!(error = ?err, "failed to start turn through app server");
|
||||
}
|
||||
}
|
||||
AppEvent::RetrySafetyBufferedTurn {
|
||||
thread_id,
|
||||
|
||||
@@ -10,6 +10,8 @@ mod safety_buffering;
|
||||
mod session_lifecycle_requests;
|
||||
mod session_summary;
|
||||
mod startup;
|
||||
#[path = "tests/turn_submission.rs"]
|
||||
mod turn_submission;
|
||||
|
||||
use super::*;
|
||||
use crate::app_backtrack::BacktrackSelection;
|
||||
|
||||
@@ -0,0 +1,52 @@
|
||||
use super::*;
|
||||
use crate::chatwidget::UserMessage;
|
||||
|
||||
#[tokio::test]
|
||||
async fn turn_start_failure_is_shown_without_exiting() -> Result<()> {
|
||||
let (mut app, mut app_event_rx, mut op_rx) = make_test_app_with_channels().await;
|
||||
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||
let mut app_server = Box::pin(crate::start_embedded_app_server_for_picker(&app.config)).await?;
|
||||
let thread_id = ThreadId::from_string("123e4567-e89b-12d3-a456-426614174000")?;
|
||||
app.active_thread_id = Some(thread_id);
|
||||
app.chat_widget
|
||||
.handle_thread_session(test_thread_session(thread_id, app.config.cwd.to_path_buf()));
|
||||
while app_event_rx.try_recv().is_ok() {}
|
||||
|
||||
app.chat_widget
|
||||
.restore_user_message_to_composer(UserMessage::from("hello"));
|
||||
app.chat_widget
|
||||
.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
|
||||
let op = next_user_turn_op(&mut op_rx);
|
||||
while let Ok(event) = app_event_rx.try_recv() {
|
||||
app.handle_event(&mut tui, &mut app_server, event).await?;
|
||||
}
|
||||
|
||||
let control = app
|
||||
.handle_event(&mut tui, &mut app_server, AppEvent::CodexOp(op))
|
||||
.await?;
|
||||
|
||||
assert!(matches!(control, AppRunControl::Continue));
|
||||
let error_cell = std::iter::from_fn(|| app_event_rx.try_recv().ok())
|
||||
.find_map(|event| match event {
|
||||
AppEvent::InsertHistoryCell(cell) => Some(cell),
|
||||
_ => None,
|
||||
})
|
||||
.expect("turn/start failure should be added to history");
|
||||
let transcript = app
|
||||
.transcript_cells
|
||||
.iter()
|
||||
.map(|cell| lines_to_single_string(&cell.display_lines(/*width*/ 80)))
|
||||
.chain(std::iter::once(lines_to_single_string(
|
||||
&error_cell.display_lines(/*width*/ 80),
|
||||
)))
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
insta::assert_snapshot!(transcript, @r"
|
||||
› hello
|
||||
|
||||
■ Failed to start turn: turn/start failed in TUI: turn/start failed: thread not found: 123e4567-e89b-12d3-a456-426614174000 (code -32600)
|
||||
");
|
||||
|
||||
app_server.shutdown().await?;
|
||||
Ok(())
|
||||
}
|
||||
@@ -366,7 +366,7 @@ impl ChatWidget {
|
||||
self.maybe_send_next_queued_input();
|
||||
}
|
||||
|
||||
pub(super) fn on_error(&mut self, message: String) {
|
||||
fn on_error(&mut self, message: String) {
|
||||
self.input_queue.submit_pending_steers_after_interrupt = false;
|
||||
self.flush_answer_stream_with_separator();
|
||||
self.finalize_turn();
|
||||
@@ -381,6 +381,14 @@ impl ChatWidget {
|
||||
self.maybe_send_next_queued_input();
|
||||
}
|
||||
|
||||
pub(crate) fn handle_turn_start_rejection(&mut self, message: String) -> bool {
|
||||
if !self.input_queue.user_turn_pending_start {
|
||||
return false;
|
||||
}
|
||||
self.on_error(message);
|
||||
true
|
||||
}
|
||||
|
||||
pub(super) fn on_cyber_policy_error(&mut self) {
|
||||
self.input_queue.submit_pending_steers_after_interrupt = false;
|
||||
self.finalize_turn();
|
||||
|
||||
@@ -165,6 +165,8 @@ where
|
||||
pub last_known_cursor_pos: Position,
|
||||
/// Count of visible history rows rendered above the viewport in inline mode.
|
||||
visible_history_rows: u16,
|
||||
#[cfg(test)]
|
||||
screen_size_override: Option<Size>,
|
||||
}
|
||||
|
||||
impl<B> Drop for Terminal<B>
|
||||
@@ -242,6 +244,8 @@ where
|
||||
last_known_screen_size: screen_size,
|
||||
last_known_cursor_pos: cursor_pos,
|
||||
visible_history_rows: 0,
|
||||
#[cfg(test)]
|
||||
screen_size_override: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -251,7 +255,10 @@ where
|
||||
screen_size: Size,
|
||||
cursor_pos: Position,
|
||||
) -> Self {
|
||||
Self::with_screen_size_and_cursor_position(backend, screen_size, cursor_pos)
|
||||
let mut terminal =
|
||||
Self::with_screen_size_and_cursor_position(backend, screen_size, cursor_pos);
|
||||
terminal.screen_size_override = Some(screen_size);
|
||||
terminal
|
||||
}
|
||||
|
||||
/// Get a Frame object which provides a consistent view into the terminal state for rendering.
|
||||
@@ -550,6 +557,10 @@ where
|
||||
|
||||
/// Queries the real size of the backend.
|
||||
pub fn size(&self) -> io::Result<Size> {
|
||||
#[cfg(test)]
|
||||
if let Some(size) = self.screen_size_override {
|
||||
return Ok(size);
|
||||
}
|
||||
self.backend.size()
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user