From 850f035b8c7efe8c2ed33f2952c7252bff4fce93 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Mon, 27 Apr 2026 13:02:39 -0700 Subject: [PATCH 01/12] Fix filtered thread-list resume regression in TUI (#19591) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why `codex resume` regressed after [#18502](https://github.com/openai/codex/pull/18502) changed the default `thread/list` scan-and-repair path for metadata-filtered listings. The TUI resume picker uses `thread/list` with source/provider/cwd filters and `useStateDbOnly: false`, which is the intended correctness-preserving mode: it should still consult the filesystem so healthy, missing, or stale SQLite state can be repaired. The regression was that #18502 made that filtered, filesystem-backed path call `reconcile_rollout` for every filesystem hit, and then call it again for each SQLite hit. When `reconcile_rollout` does not already have extracted rollout items, it falls back to loading the full JSONL rollout. That changed the resume picker’s first page from a cheap rollout-head scan plus SQLite read-repair into full-file reads for large sessions, so a few long threads could dominate TUI startup/resume latency. This change addresses the regression by keeping `useStateDbOnly: false` on the correctness-preserving path while avoiding unnecessary full JSONL reads for rows the filesystem scan has already validated. Source/provider/cwd filters can be decided from rollout-head metadata, so non-search resume listings only need the lightweight read-repair path for filesystem hits. Full reconciliation is still used for DB-only filtered rows because those can be stale false positives, and for search listings because search can depend on title metadata that may require scanning the full rollout. This fixes #19483. ## What changed - For non-search filtered listings, repair filesystem hits with the lightweight `read_repair_rollout_path` path instead of full `reconcile_rollout`. - Track thread IDs proven by the filesystem scan and only fully reconcile SQLite-filtered hits that the filesystem scan did not return, preserving stale-DB false-positive cleanup without full-reading every healthy rollout. - Leave search listings on full reconciliation, since search depends on full title metadata rather than only source/provider/cwd metadata from the rollout head. ## Verification - `cargo test -p codex-rollout list_threads` - `cargo test -p codex-app-server thread_list` --- codex-rs/rollout/src/recorder.rs | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/codex-rs/rollout/src/recorder.rs b/codex-rs/rollout/src/recorder.rs index 7b3037c4a9..0a9ae62246 100644 --- a/codex-rs/rollout/src/recorder.rs +++ b/codex-rs/rollout/src/recorder.rs @@ -444,9 +444,19 @@ impl RolloutRecorder { )); } - // Warm the DB by repairing every filesystem hit before querying SQLite. + // For metadata-filtered listings the filesystem page is the page we return. Track those + // IDs so the later DB page only triggers full reconciliation for DB-only hits. + let fs_page_thread_ids = fs_page + .items + .iter() + .filter_map(|item| item.thread_id) + .collect::>(); + + // Warm the DB by repairing every filesystem hit before querying SQLite. Source/provider/cwd + // filters are already validated from rollout head metadata, so lightweight read-repair is + // enough there. Search can depend on full title metadata, so keep full reconciliation. for item in &fs_page.items { - if listing_has_metadata_filters { + if search_term.is_some() { state_db::reconcile_rollout( state_db_ctx.as_deref(), item.path.as_path(), @@ -517,6 +527,12 @@ impl RolloutRecorder { } if listing_has_metadata_filters { for item in &db_page.items { + // Rows that also appeared in the filesystem page were just validated from the + // rollout head. Rows only found by SQLite may be stale filter matches, so fully + // reconcile those before returning the filesystem-backed page. + if fs_page_thread_ids.contains(&item.id) { + continue; + } state_db::reconcile_rollout( state_db_ctx.as_deref(), item.rollout_path.as_path(), From 0bd25ab374eba97072a733f496cc7adec9ad8955 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Mon, 27 Apr 2026 13:20:55 -0700 Subject: [PATCH 02/12] Delay approval prompts while typing (#19513) ## Why Fixes #7744. Approval modals can currently appear while the user is typing ahead in the TUI composer, which lets plain letters like `y` or `a` get consumed as approval shortcuts instead of staying in the draft input. ## What changed - Track recent composer typing activity in `bottom_pane/mod.rs`. - Delay new approval overlays for 1 second while the composer is active, keeping delayed requests queued until the user is idle. - Preserve the existing active-overlay behavior so approvals that arrive while an approval modal is already open are still queued into that overlay. - Prune delayed approvals when app-server resolution says the request has already been handled. ## Verification Added unit coverage for immediate approvals, delayed approvals, idle deadline reset, typed shortcut letters staying in the composer, shortcut handling after the delay, and resolved delayed-request pruning. Focused `codex-tui` test groups pass locally. The full `cargo test -p codex-tui` run currently aborts in `app::tests::attach_live_thread_for_selection_rejects_unmaterialized_fallback_threads`; that same test also fails when run alone with the same stack overflow. Manual reviewer check: 1. Start the TUI from the repo root: ```bash RUST_LOG=trace just codex \ -c log_dir= \ --ask-for-approval untrusted \ --sandbox workspace-write ``` 2. Submit this prompt: ```text create a file text.txt on my desktop ``` 3. While the agent is preparing the approval request, immediately type text such as `ya this should stay in the composer`. 4. Confirm the typed-ahead `y`/`a` remains in the composer instead of approving the request. 5. Stop typing for about 1 second; the approval modal should then appear. 6. Once the modal is visible, press `y` and confirm the approval shortcut works normally. --- .../tui/src/bottom_pane/approval_overlay.rs | 2 +- codex-rs/tui/src/bottom_pane/mod.rs | 251 +++++++++++++++++- 2 files changed, 245 insertions(+), 8 deletions(-) diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index ce3b04b33b..bb7e6bd8bb 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -102,7 +102,7 @@ impl ApprovalRequest { } } - fn matches_resolved_request(&self, request: &ResolvedAppServerRequest) -> bool { + pub(super) fn matches_resolved_request(&self, request: &ResolvedAppServerRequest) -> bool { match (self, request) { ( ApprovalRequest::Exec { id, .. }, diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index a2067fb7e7..7defaaf6ff 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -13,6 +13,7 @@ //! //! Some UI is time-based rather than input-based, such as the transient "press again to quit" //! hint. The pane schedules redraws so those hints can expire even when the UI is otherwise idle. +use std::collections::VecDeque; use std::path::PathBuf; use crate::app::app_server_requests::ResolvedAppServerRequest; @@ -42,6 +43,7 @@ use ratatui::buffer::Buffer; use ratatui::layout::Rect; use ratatui::text::Line; use std::time::Duration; +use std::time::Instant; mod app_link_view; mod approval_overlay; @@ -139,6 +141,8 @@ pub(crate) use selection_tabs::SelectionTab; /// Keeping a single value ensures Ctrl+C and Ctrl+D behave identically. pub(crate) const QUIT_SHORTCUT_TIMEOUT: Duration = Duration::from_secs(1); +const APPROVAL_PROMPT_TYPING_IDLE_DELAY: Duration = Duration::from_secs(1); + /// Whether Ctrl+C/Ctrl+D require a second press to quit. /// /// This UX experiment was enabled by default, but requiring a double press to quit feels janky in @@ -170,6 +174,11 @@ pub(crate) use experimental_features_view::ExperimentalFeaturesView; pub(crate) use list_selection_view::SelectionAction; pub(crate) use list_selection_view::SelectionItem; +struct DelayedApprovalRequest { + request: ApprovalRequest, + features: Features, +} + /// Pane displayed in the lower half of the chat UI. /// /// This is the owning container for the prompt input (`ChatComposer`) and the view stack @@ -182,6 +191,8 @@ pub(crate) struct BottomPane { /// Stack of views displayed instead of the composer (e.g. popups/modals). view_stack: Vec>, + delayed_approval_requests: VecDeque, + last_composer_activity_at: Option, app_event_tx: AppEventSender, frame_requester: FrameRequester, @@ -243,6 +254,8 @@ impl BottomPane { Self { composer, view_stack: Vec::new(), + delayed_approval_requests: VecDeque::new(), + last_composer_activity_at: None, app_event_tx, frame_requester, has_input_focus, @@ -442,6 +455,48 @@ impl BottomPane { } } + fn approval_prompt_delay_remaining(&self, now: Instant) -> Option { + self.last_composer_activity_at.and_then(|last_activity_at| { + last_activity_at + .checked_add(APPROVAL_PROMPT_TYPING_IDLE_DELAY) + .and_then(|show_at| show_at.checked_duration_since(now)) + .filter(|delay| !delay.is_zero()) + }) + } + + fn record_composer_activity_at(&mut self, now: Instant) { + self.last_composer_activity_at = Some(now); + if !self.delayed_approval_requests.is_empty() + && let Some(delay) = self.approval_prompt_delay_remaining(now) + { + self.request_redraw_in(delay); + } + } + + fn maybe_show_delayed_approval_requests_at(&mut self, now: Instant) { + if self.delayed_approval_requests.is_empty() || !self.view_stack.is_empty() { + return; + } + if let Some(delay) = self.approval_prompt_delay_remaining(now) { + self.request_redraw_in(delay); + return; + } + + // Promote the oldest delayed approval once typing has been idle long enough. + // `ApprovalOverlay` advances its internal queue with `pop()`, so drain the + // remaining delayed approvals from the back to preserve FIFO display order. + let Some(first) = self.delayed_approval_requests.pop_front() else { + return; + }; + let mut modal = + ApprovalOverlay::new(first.request, self.app_event_tx.clone(), first.features); + while let Some(delayed) = self.delayed_approval_requests.pop_back() { + modal.enqueue_request(delayed.request); + } + self.pause_status_timer_for_modal(); + self.push_view(Box::new(modal)); + } + /// Forward a key event to the active view or the composer. pub fn handle_key_event(&mut self, key_event: KeyEvent) -> InputResult { // If a modal/view is active, handle it here; otherwise forward to composer. @@ -512,7 +567,21 @@ impl BottomPane { self.request_redraw(); return InputResult::None; } + let records_composer_activity = + matches!(key_event.kind, KeyEventKind::Press | KeyEventKind::Repeat) + && !key_hint::has_ctrl_or_alt(key_event.modifiers) + && matches!( + key_event.code, + KeyCode::Char(_) + | KeyCode::Backspace + | KeyCode::Delete + | KeyCode::Enter + | KeyCode::Tab + ); let (input_result, needs_redraw) = self.composer.handle_key_event(key_event); + if records_composer_activity { + self.record_composer_activity_at(Instant::now()); + } if needs_redraw { self.request_redraw(); } @@ -560,6 +629,7 @@ impl BottomPane { } pub fn handle_paste(&mut self, pasted: String) { + let has_pasted_text = !pasted.is_empty(); if let Some(view) = self.view_stack.last_mut() { let needs_redraw = view.handle_paste(pasted); let view_complete = view.is_complete(); @@ -572,6 +642,9 @@ impl BottomPane { } } else { let needs_redraw = self.composer.handle_paste(pasted); + if has_pasted_text { + self.record_composer_activity_at(Instant::now()); + } self.composer.sync_popups(); if needs_redraw { self.request_redraw(); @@ -586,7 +659,12 @@ impl BottomPane { } pub(crate) fn pre_draw_tick(&mut self) { + self.pre_draw_tick_at(Instant::now()); + } + + fn pre_draw_tick_at(&mut self, now: Instant) { self.composer.sync_popups(); + self.maybe_show_delayed_approval_requests_at(now); } /// Replace the composer text with `text`. @@ -989,10 +1067,22 @@ impl BottomPane { request }; - // Otherwise create a new approval modal overlay. - let modal = ApprovalOverlay::new(request, self.app_event_tx.clone(), features.clone()); - self.pause_status_timer_for_modal(); - self.push_view(Box::new(modal)); + let now = Instant::now(); + if !self.delayed_approval_requests.is_empty() + || self.approval_prompt_delay_remaining(now).is_some() + { + self.delayed_approval_requests + .push_back(DelayedApprovalRequest { + request, + features: features.clone(), + }); + self.maybe_show_delayed_approval_requests_at(now); + } else { + // No recent composer activity, so show the approval modal immediately. + let modal = ApprovalOverlay::new(request, self.app_event_tx.clone(), features.clone()); + self.pause_status_timer_for_modal(); + self.push_view(Box::new(modal)); + } } /// Called when the agent requests user input. @@ -1107,11 +1197,19 @@ impl BottomPane { &mut self, request: &ResolvedAppServerRequest, ) -> bool { + let delayed_len = self.delayed_approval_requests.len(); + self.delayed_approval_requests + .retain(|delayed| !delayed.request.matches_resolved_request(request)); + let delayed_changed = self.delayed_approval_requests.len() != delayed_len; + if self.view_stack.is_empty() { - return false; + if delayed_changed { + self.request_redraw(); + } + return delayed_changed; } - let mut changed = false; + let mut changed = delayed_changed; let mut completed_indices = Vec::new(); for index in (0..self.view_stack.len()).rev() { let view = &mut self.view_stack[index]; @@ -1356,6 +1454,7 @@ mod tests { use crate::test_support::PathBufExt; use crate::test_support::test_path_buf; use codex_protocol::protocol::Op; + use codex_protocol::protocol::ReviewDecision; use codex_protocol::protocol::SkillScope; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; @@ -1367,6 +1466,7 @@ mod tests { use ratatui::layout::Rect; use std::cell::Cell; use std::rc::Rc; + use std::time::Instant; use tokio::sync::mpsc::unbounded_channel; fn snapshot_buffer(buf: &Buffer) -> String { @@ -1388,13 +1488,20 @@ mod tests { } fn test_pane(app_event_tx: AppEventSender) -> BottomPane { + test_pane_with_disable_paste_burst(app_event_tx, /*disable_paste_burst*/ false) + } + + fn test_pane_with_disable_paste_burst( + app_event_tx: AppEventSender, + disable_paste_burst: bool, + ) -> BottomPane { BottomPane::new(BottomPaneParams { app_event_tx, frame_requester: FrameRequester::test_dummy(), has_input_focus: true, enhanced_keys_supported: false, placeholder_text: "Ask Codex to do anything".to_string(), - disable_paste_burst: false, + disable_paste_burst, animations_enabled: true, skills: Some(Vec::new()), }) @@ -1565,6 +1672,136 @@ mod tests { ); } + #[test] + fn approval_request_shows_immediately_without_recent_typing() { + let (tx_raw, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let features = Features::with_defaults(); + let mut pane = test_pane(tx); + + pane.push_approval_request(exec_request(), &features); + + assert_eq!(pane.view_stack.len(), 1); + assert!(pane.delayed_approval_requests.is_empty()); + } + + #[test] + fn approval_request_is_delayed_after_recent_typing() { + let (tx_raw, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let features = Features::with_defaults(); + let mut pane = test_pane(tx); + let now = Instant::now(); + pane.last_composer_activity_at = Some(now); + + pane.push_approval_request(exec_request(), &features); + + assert!(pane.view_stack.is_empty()); + assert_eq!(pane.delayed_approval_requests.len(), 1); + + pane.pre_draw_tick_at( + now + APPROVAL_PROMPT_TYPING_IDLE_DELAY - Duration::from_millis(/*millis*/ 1), + ); + assert!(pane.view_stack.is_empty()); + assert_eq!(pane.delayed_approval_requests.len(), 1); + + pane.pre_draw_tick_at(now + APPROVAL_PROMPT_TYPING_IDLE_DELAY); + assert_eq!(pane.view_stack.len(), 1); + assert!(pane.delayed_approval_requests.is_empty()); + } + + #[test] + fn continued_typing_resets_delayed_approval_idle_deadline() { + let (tx_raw, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let features = Features::with_defaults(); + let mut pane = test_pane(tx); + let first_activity = Instant::now(); + pane.last_composer_activity_at = Some(first_activity); + pane.push_approval_request(exec_request(), &features); + + let continued_activity = first_activity + Duration::from_millis(/*millis*/ 750); + pane.record_composer_activity_at(continued_activity); + + pane.pre_draw_tick_at(first_activity + APPROVAL_PROMPT_TYPING_IDLE_DELAY); + assert!(pane.view_stack.is_empty()); + assert_eq!(pane.delayed_approval_requests.len(), 1); + + pane.pre_draw_tick_at(continued_activity + APPROVAL_PROMPT_TYPING_IDLE_DELAY); + assert_eq!(pane.view_stack.len(), 1); + assert!(pane.delayed_approval_requests.is_empty()); + } + + #[test] + fn typed_approval_shortcuts_during_delay_stay_in_composer() { + let (tx_raw, mut rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let features = Features::with_defaults(); + let mut pane = test_pane_with_disable_paste_burst(tx, /*disable_paste_burst*/ true); + pane.last_composer_activity_at = Some(Instant::now()); + pane.push_approval_request(exec_request(), &features); + + pane.handle_key_event(KeyEvent::new(KeyCode::Char('y'), KeyModifiers::NONE)); + pane.handle_key_event(KeyEvent::new(KeyCode::Char('a'), KeyModifiers::NONE)); + + assert_eq!(pane.composer_text(), "ya"); + assert!(pane.view_stack.is_empty()); + assert_eq!(pane.delayed_approval_requests.len(), 1); + while let Ok(event) = rx.try_recv() { + assert!( + !matches!(event, AppEvent::SubmitThreadOp { .. }), + "delayed approval shortcut should not submit an approval: {event:?}" + ); + } + } + + #[test] + fn delayed_approval_shortcut_works_after_idle_deadline() { + let (tx_raw, mut rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let features = Features::with_defaults(); + let mut pane = test_pane(tx); + let now = Instant::now(); + pane.last_composer_activity_at = Some(now); + pane.push_approval_request(exec_request(), &features); + + pane.pre_draw_tick_at(now + APPROVAL_PROMPT_TYPING_IDLE_DELAY); + pane.handle_key_event(KeyEvent::new(KeyCode::Char('y'), KeyModifiers::NONE)); + + let mut approval_decision = None; + while let Ok(event) = rx.try_recv() { + if let AppEvent::SubmitThreadOp { + op: Op::ExecApproval { decision, .. }, + .. + } = event + { + approval_decision = Some(decision); + } + } + assert_eq!(approval_decision, Some(ReviewDecision::Approved)); + } + + #[test] + fn dismiss_app_server_request_prunes_delayed_approval() { + let (tx_raw, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let features = Features::with_defaults(); + let mut pane = test_pane(tx); + let now = Instant::now(); + pane.last_composer_activity_at = Some(now); + pane.push_approval_request(exec_request(), &features); + + assert!( + pane.dismiss_app_server_request(&ResolvedAppServerRequest::ExecApproval { + id: "1".to_string(), + }) + ); + assert!(pane.delayed_approval_requests.is_empty()); + + pane.pre_draw_tick_at(now + APPROVAL_PROMPT_TYPING_IDLE_DELAY); + assert!(pane.view_stack.is_empty()); + } + #[test] fn dismiss_app_server_request_removes_matching_buried_view() { let (tx_raw, _rx) = unbounded_channel::(); From 52c06b8759eed97b2335e1046b3d7c8e4ddafb81 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Mon, 27 Apr 2026 13:40:46 -0700 Subject: [PATCH 03/12] Preserve TUI markdown list spacing after code blocks (#19706) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why Fixes #19702. The TUI markdown renderer could visually attach the next list marker to a fenced code block inside the previous list item, even when the source markdown included a blank line before the next item. That made block-heavy loose lists harder to read, while the desired behavior is still to keep simple lists compact. ## What changed - Track whether the current rendered list item contains a code block. - Preserve one blank separator before the following list marker only when the previous item contained a code block. - Add regression coverage for both paths: code-block list items keep the separator, and simple loose list items stay compact. ## Verification - `cargo test -p codex-tui markdown_render` I also manually verified that the bug exists before and is fixed after. ## Before Screenshot 2026-04-26 at 1 19 01 PM ## After Screenshot 2026-04-26 at 1 18 54 PM --- codex-rs/tui/src/markdown_render.rs | 23 ++++++++ codex-rs/tui/src/markdown_render_tests.rs | 52 +++++++++++++++++++ ...fter_code_block_keeps_blank_separator.snap | 9 ++++ 3 files changed, 84 insertions(+) create mode 100644 codex-rs/tui/src/snapshots/codex_tui__markdown_render__markdown_render_tests__list_item_after_code_block_keeps_blank_separator.snap diff --git a/codex-rs/tui/src/markdown_render.rs b/codex-rs/tui/src/markdown_render.rs index ae680f5db3..3dfef1d9d8 100644 --- a/codex-rs/tui/src/markdown_render.rs +++ b/codex-rs/tui/src/markdown_render.rs @@ -154,6 +154,8 @@ where inline_styles: Vec