From 57611028684594a098edf15610daa47b27922cc7 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Wed, 16 Sep 2026 16:27:04 +0000 Subject: [PATCH] Keep selection adjacent when hiding tasks in the agents overview (#45978) ## Why Hiding the selected task can reset selection to the current task or first row, interrupting navigation through the list. ## What changed Select the next visible task before hiding the selected task, falling back to the previous task at the end of the list. Follow the active grouping and search filter. Preserve the existing rename cancellation behavior when hiding a rename target so its draft cannot transfer to a neighboring task. ## Testing Add regression tests and a selection snapshot covering project, status, and model grouping, filtered and unfiltered lists, custom hide bindings, hiding all visible tasks, and hiding a task with an unsubmitted rename draft. GitOrigin-RevId: e5ca6b2a9955622bcf60e68621322b3abd801a7f --- .../src/app/agents_overview_actions_tests.rs | 166 ++++++++++++++++++ codex-rs/tui/src/app/agents_overview_view.rs | 16 +- ...s_selection_adjacent_in_display_order.snap | 26 +++ 3 files changed, 205 insertions(+), 3 deletions(-) create mode 100644 codex-rs/tui/src/app/snapshots/codex_tui__app__agents_overview__tests__actions__hiding_tasks_keeps_selection_adjacent_in_display_order.snap diff --git a/codex-rs/tui/src/app/agents_overview_actions_tests.rs b/codex-rs/tui/src/app/agents_overview_actions_tests.rs index 94f53e63ec..835a4a4367 100644 --- a/codex-rs/tui/src/app/agents_overview_actions_tests.rs +++ b/codex-rs/tui/src/app/agents_overview_actions_tests.rs @@ -108,6 +108,172 @@ async fn lifecycle_shortcuts_target_filtered_task_in_any_state() { } } +#[tokio::test] +async fn hiding_tasks_keeps_selection_adjacent_in_display_order() -> Result<()> { + let (mut app, mut rx, _op_rx) = crate::app::tests::make_test_app_with_channels().await; + let mut app_server = crate::start_embedded_app_server_for_picker(&app.config).await?; + let mut tui = crate::tui::test_support::make_test_tui()?; + let threads = (1..=4) + .map(|index| { + let mut thread = overview_thread( + ThreadId::from_u128(index), + /*parent_thread_id*/ None, + if index == 2 { "Other" } else { "Task" }, + ThreadStatus::Idle, + ); + thread.name = Some(format!("{} {index}", thread.preview)); + thread.cwd = test_path_buf(&format!("/tmp/project-{index}")).abs(); + thread.model = Some(format!("model-{index}")); + thread.updated_at = index as i64; + thread + }) + .collect::>(); + app.primary_thread_id = Some(ThreadId::from_u128(/*value*/ 1)); + app.agents_overview.threads = threads + .iter() + .map(|thread| { + ( + ThreadId::from_string(&thread.id).unwrap(), + Some(thread.clone()), + ) + }) + .collect(); + let mut selections = Vec::new(); + for grouping in [ + AgentsOverviewGrouping::Project, + AgentsOverviewGrouping::Status, + AgentsOverviewGrouping::Model, + ] { + for filtered in [false, true] { + app.agents_overview.hidden_threads.clear(); + app.agents_overview.view_state.lock().unwrap().grouping = grouping; + let mut keymap = TuiKeymap::default(); + let hide_key = if filtered { + KeyCode::F(7) + } else { + KeyCode::Char('h') + }; + if filtered { + keymap.agents.hide = Some(KeybindingsSpec::One(KeybindingSpec("f7".into()))); + } + app.keymap = RuntimeKeymap::from_config(&keymap).unwrap(); + let mut view = + app.agents_overview_view(threads.clone(), Some(ThreadId::from_u128(/*value*/ 3))); + view.handle_key_event(KeyCode::Esc.into()); + if filtered { + view.handle_key_event(KeyCode::Char('f').into()); + view.handle_paste("Task".into()); + // Search selects the first match; move back to Task 3. + view.handle_key_event(KeyCode::Down.into()); + } + app.agents_overview.visible_thread_ids = view.thread_ids(); + app.chat_widget.show_bottom_pane_view(Box::new(view)); + let expected = match (grouping, filtered) { + (AgentsOverviewGrouping::Status, false) => vec![3, 2, 1, 4], + (AgentsOverviewGrouping::Status, true) => vec![3, 1, 4], + (AgentsOverviewGrouping::Project | AgentsOverviewGrouping::Model, false) => { + vec![3, 4, 2, 1] + } + (AgentsOverviewGrouping::Project | AgentsOverviewGrouping::Model, true) => { + vec![3, 4, 1] + } + }; + for index in expected { + let selected = app + .chat_widget + .selected_index_for_present_view(AGENTS_OVERVIEW_VIEW_ID) + .unwrap(); + assert_eq!( + app.agents_overview.visible_thread_ids[selected], + ThreadId::from_u128(index), + "{grouping:?}, filtered={filtered}" + ); + let rendered = render_bottom_popup(&app.chat_widget, /*width*/ 100); + let selected_row = rendered.lines().find(|line| line.contains('›')).unwrap(); + selections.push(format!( + "{grouping:?}, filtered={filtered}: {}", + selected_row.split('│').next().unwrap().trim() + )); + app.chat_widget.handle_key_event(hide_key.into()); + let hide = std::iter::from_fn(|| rx.try_recv().ok()) + .find(|event| matches!(event, AppEvent::HideAgentsOverviewThread { .. })) + .expect("hide shortcut emits an event"); + assert!(matches!( + &hide, + AppEvent::HideAgentsOverviewThread { thread_id } + if *thread_id == ThreadId::from_u128(index) + )); + Box::pin(app.handle_event(&mut tui, &mut app_server, hide)).await?; + } + let selected = app + .chat_widget + .selected_index_for_present_view(AGENTS_OVERVIEW_VIEW_ID) + .unwrap(); + assert_eq!(app.agents_overview.visible_thread_ids.get(selected), None); + app.chat_widget.handle_key_event(hide_key.into()); + assert!( + !std::iter::from_fn(|| rx.try_recv().ok()) + .any(|event| matches!(event, AppEvent::HideAgentsOverviewThread { .. })) + ); + } + } + insta::assert_snapshot!(selections.join("\n")); + app_server.shutdown().await?; + Ok(()) +} + +#[tokio::test] +async fn hiding_rename_target_does_not_transfer_draft_to_neighbor() -> Result<()> { + let (mut app, mut rx, _op_rx) = crate::app::tests::make_test_app_with_channels().await; + let mut app_server = crate::start_embedded_app_server_for_picker(&app.config).await?; + let mut tui = crate::tui::test_support::make_test_tui()?; + let mut keymap = TuiKeymap::default(); + keymap.agents.hide = Some(KeybindingsSpec::One(KeybindingSpec("f7".into()))); + app.keymap = RuntimeKeymap::from_config(&keymap).unwrap(); + let threads = ["Rename target", "Neighbor"] + .into_iter() + .map(|name| { + overview_thread( + ThreadId::new(), + /*parent_thread_id*/ None, + name, + ThreadStatus::Idle, + ) + }) + .collect::>(); + let target = ThreadId::from_string(&threads[0].id)?; + app.agents_overview.threads = threads + .iter() + .map(|thread| { + ( + ThreadId::from_string(&thread.id).unwrap(), + Some(thread.clone()), + ) + }) + .collect(); + let mut view = app.agents_overview_view(threads, Some(target)); + view.handle_key_event(KeyCode::Char('r').into()); + view.handle_paste("Unsubmitted title".into()); + app.agents_overview.visible_thread_ids = view.thread_ids(); + app.chat_widget.show_bottom_pane_view(Box::new(view)); + app.chat_widget.handle_key_event(KeyCode::F(7).into()); + let hide = std::iter::from_fn(|| rx.try_recv().ok()) + .find(|event| matches!(event, AppEvent::HideAgentsOverviewThread { .. })) + .expect("hide shortcut emits an event"); + Box::pin(app.handle_event(&mut tui, &mut app_server, hide)).await?; + { + let state = app.agents_overview.view_state.lock().unwrap(); + assert_eq!((state.renaming, state.input.as_str()), (false, "")); + } + app.chat_widget.handle_key_event(KeyCode::Enter.into()); + assert!( + !std::iter::from_fn(|| rx.try_recv().ok()) + .any(|event| matches!(event, AppEvent::RenameAgentsOverviewThread { .. })) + ); + app_server.shutdown().await?; + Ok(()) +} + #[tokio::test] async fn hidden_task_stays_hidden_through_activity_and_seed_until_explicit_resume() -> Result<()> { let (mut app, mut rx, _op_rx) = crate::app::tests::make_test_app_with_channels().await; diff --git a/codex-rs/tui/src/app/agents_overview_view.rs b/codex-rs/tui/src/app/agents_overview_view.rs index 5965b6516a..fa24566045 100644 --- a/codex-rs/tui/src/app/agents_overview_view.rs +++ b/codex-rs/tui/src/app/agents_overview_view.rs @@ -655,9 +655,19 @@ impl BottomPaneView for AgentsOverviewView { } if self.agents_keymap.hide.is_pressed(key) { if let Some(row) = self.selected_row() { - self.app_event_tx.send(AppEvent::HideAgentsOverviewThread { - thread_id: row.thread_id, - }); + let thread_id = row.thread_id; + let visible = self.visible_indices(); + if !self.state().renaming + && let Some(position) = visible.iter().position(|index| *index == self.selected) + && let Some(next) = visible + .get(position + 1) + .or_else(|| visible.get(position.saturating_sub(1))) + { + // Preserve a neighboring row when hiding rebuilds the view. + self.selected = *next; + } + self.app_event_tx + .send(AppEvent::HideAgentsOverviewThread { thread_id }); } return; } diff --git a/codex-rs/tui/src/app/snapshots/codex_tui__app__agents_overview__tests__actions__hiding_tasks_keeps_selection_adjacent_in_display_order.snap b/codex-rs/tui/src/app/snapshots/codex_tui__app__agents_overview__tests__actions__hiding_tasks_keeps_selection_adjacent_in_display_order.snap new file mode 100644 index 0000000000..d012ada0fd --- /dev/null +++ b/codex-rs/tui/src/app/snapshots/codex_tui__app__agents_overview__tests__actions__hiding_tasks_keeps_selection_adjacent_in_display_order.snap @@ -0,0 +1,26 @@ +--- +source: tui/src/app/agents_overview_actions_tests.rs +assertion_line: 211 +expression: "selections.join(\"\\n\")" +--- +Project, filtered=false: › ○ Task 3 Ready +Project, filtered=false: › ○ Task 4 Ready +Project, filtered=false: › ○ Other 2 Ready +Project, filtered=false: › ○ Task 1 current Ready +Project, filtered=true: › ○ Task 3 Ready +Project, filtered=true: › ○ Task 4 Ready +Project, filtered=true: › ○ Task 1 current Ready +Status, filtered=false: › ○ Task 3 +Status, filtered=false: › ○ Other 2 +Status, filtered=false: › ○ Task 1 current +Status, filtered=false: › ○ Task 4 +Status, filtered=true: › ○ Task 3 +Status, filtered=true: › ○ Task 1 current +Status, filtered=true: › ○ Task 4 +Model, filtered=false: › ○ Task 3 Ready +Model, filtered=false: › ○ Task 4 Ready +Model, filtered=false: › ○ Other 2 Ready +Model, filtered=false: › ○ Task 1 current Ready +Model, filtered=true: › ○ Task 3 Ready +Model, filtered=true: › ○ Task 4 Ready +Model, filtered=true: › ○ Task 1 current Ready