Keep startup modals protected when terminal input draining times out (#52968)

## Why

An unfinished terminal control sequence can prevent buffered input from draining before a startup modal appears. Propagating that timeout aborts modal handling instead of keeping the screen available for an explicit user decision.

## What changed

Treat input-drain timeouts as a reason to keep startup modal input quarantined. Discard key, paste, and mouse events while retrying the drain, and continue processing server events. Preserve this protection when another request resolves and when the agent overview is restored after reconnecting.

## Testing

Add a Unix PTY regression test covering unfinished control sequences, queued approval and overview shortcuts, unrelated request resolution, and overview reconnection. Verify that buffered input cannot answer an approval or create a task, while fresh shortcuts work after the drain completes.

GitOrigin-RevId: 476bedbd45f13a15d0019e4ce2e875406d4a53ca
This commit is contained in:
Eric Traut
2026-10-10 19:23:28 +00:00
committed by copyberry
parent 7a8a9456e9
commit 34f33e4ada
6 changed files with 227 additions and 8 deletions
+14 -3
View File
@@ -655,7 +655,8 @@ pub(crate) struct App {
pending_managed_worktree_attach: Option<Box<working_directory::ManagedWorktreeAttach>>,
/// Keeps protected screens quarantined until initialized chat receives genuine user input.
startup_protected_input_boundary: bool,
/// Keeps that boundary armed while a startup approval waits for the typing-idle timer.
/// Keeps that boundary armed while an approval waits for the typing-idle timer or a visible
/// modal's terminal input has not settled.
startup_pending_protected_request: bool,
/// Invalidates in-flight full rate-limit reads when a newer rolling hard stop arrives.
account_email_request_id: Option<uuid::Uuid>,
@@ -855,6 +856,17 @@ impl App {
self.invalidate_right_click_paste(&event);
self.finish_clipboard(tui, &event);
let event = self.finish_right_click_paste(tui, event);
if self.startup_protected_input_boundary
&& self.startup_pending_protected_request
&& self.chat_widget.has_active_modal()
&& matches!(
&event,
TuiEvent::Key(_) | TuiEvent::Paste(_) | TuiEvent::Mouse(_)
)
{
self.discard_startup_modal_input(tui)?;
return Ok(AppRunControl::Continue);
}
let idle_draw = matches!(event, TuiEvent::Draw);
if self.handle_rendered_selection_event(tui, &event)? {
return Ok(AppRunControl::Continue);
@@ -1121,8 +1133,7 @@ impl App {
&& self.chat_widget.has_active_modal()
&& self.startup_protected_input_boundary
{
tui.discard_pending_input_before_interactive_screen()?;
self.startup_pending_protected_request = false;
self.discard_startup_modal_input(tui)?;
}
if self.chat_widget.ambient_pet_image_enabled() {
let ambient_pet_area = Rect::new(
+2 -1
View File
@@ -282,7 +282,8 @@ impl App {
self.chat_widget.dismiss_app_server_request(&request);
if self.startup_pending_protected_request {
self.startup_pending_protected_request =
self.chat_widget.has_pending_protected_request();
self.chat_widget.has_active_modal()
|| self.chat_widget.has_pending_protected_request();
}
}
}
+4
View File
@@ -481,6 +481,10 @@ impl App {
let view = self.agents_overview_view(threads, selected);
self.chat_widget.show_bottom_pane_view(Box::new(view));
self.refresh_agents_overview_threads(app_server);
// The Overview survived request cleanup, but terminal typeahead may have survived too.
if self.startup_protected_input_boundary {
self.render_startup_frame(tui, app_event_rx)?;
}
}
#[cfg(any(target_os = "windows", test))]
if interrupted_windows_setup {
+15 -2
View File
@@ -174,8 +174,21 @@ impl App {
self.chat_widget.pre_draw_tick();
self.render_chat_widget_frame(tui, tui.terminal.last_known_screen_size)?;
if self.chat_widget.has_active_modal() && self.startup_protected_input_boundary {
tui.discard_pending_input_before_interactive_screen()?;
self.startup_pending_protected_request = false;
self.discard_startup_modal_input(tui)?;
}
Ok(())
}
/// Keep a visible modal closed to input if the terminal has not settled yet. The next input
/// event retries the drain instead of acting on the modal; server events can continue meanwhile.
pub(super) fn discard_startup_modal_input(&mut self, tui: &mut tui::Tui) -> Result<()> {
match tui.discard_pending_input_before_interactive_screen() {
Ok(()) => self.startup_pending_protected_request = false,
Err(err) if err.kind() == std::io::ErrorKind::TimedOut => {
self.startup_pending_protected_request = true;
tracing::warn!(%err, "keeping startup modal input quarantined");
}
Err(err) => return Err(err.into()),
}
Ok(())
}
+191
View File
@@ -777,6 +777,197 @@ async fn later_thread_approval_preserves_input_after_startup_boundary_ends() ->
Ok(())
}
#[cfg(unix)]
#[tokio::test]
async fn unattended_modals_survive_unfinished_terminal_input() -> Result<()> {
use std::os::fd::FromRawFd;
use std::process::Command;
use std::process::Stdio;
// Isolate crossterm's global parser and give its input poll a real terminal, as in
// tui_startup_tests. No key is delivered to the app before the approval.
if std::env::var_os("CODEX_UNATTENDED_MODALS_PTY").is_none() {
let mut master = -1;
let mut slave = -1;
// Safety: openpty initializes both file descriptors on success.
assert_eq!(
unsafe {
libc::openpty(
&mut master,
&mut slave,
std::ptr::null_mut(),
std::ptr::null_mut(),
std::ptr::null_mut(),
)
},
0,
);
// Safety: openpty transferred ownership of these descriptors.
let _master = unsafe { std::fs::File::from_raw_fd(master) };
let slave = unsafe { std::fs::File::from_raw_fd(slave) };
let output = Command::new(std::env::current_exe()?)
.args([
"--exact",
"app::tests::startup::unattended_modals_survive_unfinished_terminal_input",
"--nocapture",
])
.env("CODEX_UNATTENDED_MODALS_PTY", "1")
.stdin(Stdio::from(slave))
.output()?;
assert!(
output.status.success(),
"PTY child failed: {}\n{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr),
);
return Ok(());
}
let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await;
app.startup_protected_input_boundary = true;
let thread_id = ThreadId::new();
app.enqueue_primary_thread_session(
test_thread_session(thread_id, test_path_buf("/tmp/project")),
Vec::new(),
)
.await?;
while app_event_rx.try_recv().is_ok() {}
let mut tui = crate::tui::test_support::make_test_tui()?;
let mut app_server =
crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref()).await?;
crossterm::event::buffer_input(b"\x1b]10;")?;
let request = exec_approval_request(thread_id, "turn-1", "call-1", /*approval_id*/ None);
let _ = app
.pending_app_server_requests
.note_server_request(&request);
app.enqueue_primary_thread_request(request).await?;
let event = app.active_thread_rx.as_mut().unwrap().try_recv()?;
app.handle_active_thread_event(&mut tui, &mut app_server, event)
.await?;
assert!(app.chat_widget.has_active_modal());
// A separate tracked request may resolve while the visible approval's input is quarantined.
let mut other = exec_approval_request(
ThreadId::new(),
"turn-2",
"call-2",
/*approval_id*/ None,
);
let ServerRequest::CommandExecutionRequestApproval { request_id, params } = &mut other else {
unreachable!("exec approval helper returns an exec approval");
};
*request_id = AppServerRequestId::Integer(2);
let resolved = codex_app_server_protocol::ServerRequestResolvedNotification {
thread_id: params.thread_id.clone(),
request_id: request_id.clone(),
};
let _ = app.pending_app_server_requests.note_server_request(&other);
app.handle_app_server_event(
&app_server,
codex_app_server_client::AppServerEvent::ServerNotification(Box::new(
ServerNotification::ServerRequestResolved(resolved),
)),
)
.await;
// Action keys inside the unfinished sequence and immediately after its terminator are
// still typeahead. Even the first decoded key must retry the drain, not accept the prompt.
crossterm::event::buffer_input(b"1y\r\x1b\\y\r")?;
let crossterm::event::Event::Key(key) = crossterm::event::read()? else {
panic!("expected queued approval shortcut");
};
app.handle_tui_event(&mut tui, &mut app_server, TuiEvent::Key(key))
.await?;
assert!(app.chat_widget.has_active_modal());
while let Ok(event) = app_event_rx.try_recv() {
assert!(
!matches!(event, AppEvent::SubmitThreadOp { .. }),
"typeahead answered unattended approval: {event:?}"
);
}
// After the drain has settled a fresh explicit decision works normally.
app.handle_tui_event(
&mut tui,
&mut app_server,
TuiEvent::Key(KeyEvent::new(KeyCode::Char('y'), KeyModifiers::NONE)),
)
.await?;
assert!(!app.chat_widget.has_active_modal());
// Overview can open before the first real input, and survives a server reconnect.
let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await;
app.startup_protected_input_boundary = true;
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await?;
app.app_server_target = AppServerTarget::Remote {
endpoint: crate::resolve_remote_addr(&format!("ws://{}", listener.local_addr()?))?,
};
let server = tokio::spawn(async move {
let (stream, _) = listener.accept().await?;
super::disconnect::serve_reconnect_requests(
tokio_tungstenite::accept_async(stream).await?,
|request| {
std::future::ready(Some(match request.method.as_str() {
"thread/list" | "thread/loaded/list" => {
serde_json::json!({"result": {"data": [], "nextCursor": null}})
}
method => panic!("unexpected overview reconnect request: {method}"),
}))
},
)
.await
});
let view = app.agents_overview_view(Vec::new(), /*selected_thread_id*/ None);
app.chat_widget.show_bottom_pane_view(Box::new(view));
crossterm::event::buffer_input(b"\x1b]10;")?;
app.render_startup_frame(&mut tui, &app_event_rx)?;
assert!(app.begin_reconnect());
let connected = super::super::reconnect::reconnect(
app.app_server_target.clone(),
app.config.clone(),
app.local_settings.clone(),
/*thread_id*/ None,
/*remote_cwd*/ None,
app_server.thread_tool_transport(),
super::super::reconnect::ReconnectPresentation::Overview,
)
.await?;
app.finish_reconnect(
&mut tui,
&mut app_server,
&mut app_event_rx,
connected,
CODEX_CLI_VERSION,
)
.await?;
crossterm::event::buffer_input(b"1n\r\x1b\\n\r")?;
let crossterm::event::Event::Key(key) = crossterm::event::read()? else {
panic!("expected queued overview shortcut");
};
app.handle_tui_event(&mut tui, &mut app_server, TuiEvent::Key(key))
.await?;
assert!(
!std::iter::from_fn(|| app_event_rx.try_recv().ok())
.any(|event| matches!(event, AppEvent::NewAgentsOverviewSession { .. })),
"typeahead created a task in restored Overview"
);
app.handle_tui_event(
&mut tui,
&mut app_server,
TuiEvent::Key(KeyEvent::new(KeyCode::Char('n'), KeyModifiers::NONE)),
)
.await?;
assert!(
std::iter::from_fn(|| app_event_rx.try_recv().ok())
.any(|event| matches!(event, AppEvent::NewAgentsOverviewSession { .. })),
"fresh Overview shortcut should create a task"
);
app_server.shutdown().await?;
server.await??;
Ok(())
}
#[tokio::test]
async fn auto_declined_mcp_elicitations_do_not_leave_startup_quarantine_armed() {
for replay in [false, true] {
+1 -2
View File
@@ -2241,8 +2241,7 @@ impl App {
{
self.chat_widget.pre_draw_tick();
self.render_chat_widget_frame(tui, tui.terminal.last_known_screen_size)?;
tui.discard_pending_input_before_interactive_screen()?;
self.startup_pending_protected_request = false;
self.discard_startup_modal_input(tui)?;
}
if self.backtrack_render_pending {
tui.frame_requester().schedule_frame();