diff --git a/codex-rs/tui/src/bottom_pane/bottom_pane_view.rs b/codex-rs/tui/src/bottom_pane/bottom_pane_view.rs index a8561fe3d5..35165db491 100644 --- a/codex-rs/tui/src/bottom_pane/bottom_pane_view.rs +++ b/codex-rs/tui/src/bottom_pane/bottom_pane_view.rs @@ -22,6 +22,12 @@ pub(crate) trait BottomPaneView: Renderable { None } + /// Actual item index for list-based views that want to preserve selection + /// across external refreshes. + fn selected_index(&self) -> Option { + None + } + /// Handle Ctrl-C while this view is active. fn on_ctrl_c(&mut self) -> CancellationEvent { CancellationEvent::NotHandled diff --git a/codex-rs/tui/src/bottom_pane/list_selection_view.rs b/codex-rs/tui/src/bottom_pane/list_selection_view.rs index 9a50a07831..e3b3287131 100644 --- a/codex-rs/tui/src/bottom_pane/list_selection_view.rs +++ b/codex-rs/tui/src/bottom_pane/list_selection_view.rs @@ -677,6 +677,10 @@ impl BottomPaneView for ListSelectionView { self.view_id } + fn selected_index(&self) -> Option { + self.selected_actual_idx() + } + fn on_ctrl_c(&mut self) -> CancellationEvent { if let Some(cb) = &self.on_cancel { cb(&self.app_event_tx); diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 62142ed227..9aa16722af 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -802,6 +802,13 @@ impl BottomPane { true } + pub(crate) fn selected_index_for_active_view(&self, view_id: &'static str) -> Option { + self.view_stack + .last() + .filter(|view| view.view_id() == Some(view_id)) + .and_then(|view| view.selected_index()) + } + /// Update the pending-input preview shown above the composer. pub(crate) fn set_pending_input_preview( &mut self, diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 9e9d750b4a..62993b3b13 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -597,6 +597,7 @@ pub(crate) struct ChatWidget { /// currently executing. mcp_startup_status: Option>, connectors_cache: ConnectorsCacheState, + connectors_partial_snapshot: Option, connectors_prefetch_in_flight: bool, connectors_force_refetch_pending: bool, // Queue of interruptive UI events deferred during an active write cycle @@ -3147,6 +3148,7 @@ impl ChatWidget { agent_turn_running: false, mcp_startup_status: None, connectors_cache: ConnectorsCacheState::default(), + connectors_partial_snapshot: None, connectors_prefetch_in_flight: false, connectors_force_refetch_pending: false, interrupts: InterruptManager::new(), @@ -3329,6 +3331,7 @@ impl ChatWidget { agent_turn_running: false, mcp_startup_status: None, connectors_cache: ConnectorsCacheState::default(), + connectors_partial_snapshot: None, connectors_prefetch_in_flight: false, connectors_force_refetch_pending: false, interrupts: InterruptManager::new(), @@ -3503,6 +3506,7 @@ impl ChatWidget { agent_turn_running: false, mcp_startup_status: None, connectors_cache: ConnectorsCacheState::default(), + connectors_partial_snapshot: None, connectors_prefetch_in_flight: false, connectors_force_refetch_pending: false, interrupts: InterruptManager::new(), @@ -7632,6 +7636,10 @@ impl ChatWidget { return None; } + if let Some(snapshot) = &self.connectors_partial_snapshot { + return Some(snapshot.connectors.as_slice()); + } + match &self.connectors_cache { ConnectorsCacheState::Ready(snapshot) => Some(snapshot.connectors.as_slice()), _ => None, @@ -7737,7 +7745,9 @@ impl ChatWidget { } let connectors_cache = self.connectors_cache.clone(); - self.prefetch_connectors_with_options(true); + let should_force_refetch = !self.connectors_prefetch_in_flight + || matches!(connectors_cache, ConnectorsCacheState::Ready(_)); + self.prefetch_connectors_with_options(should_force_refetch); match connectors_cache { ConnectorsCacheState::Ready(snapshot) => { @@ -7750,28 +7760,51 @@ impl ChatWidget { ConnectorsCacheState::Failed(err) => { self.add_to_history(history_cell::new_error_event(err)); } - ConnectorsCacheState::Loading => { - self.add_to_history(history_cell::new_info_event( - "Apps are still loading.".to_string(), - Some("Try again in a moment.".to_string()), - )); - } - ConnectorsCacheState::Uninitialized => { - self.add_to_history(history_cell::new_info_event( - "Apps are still loading.".to_string(), - Some("Try again in a moment.".to_string()), - )); + ConnectorsCacheState::Loading | ConnectorsCacheState::Uninitialized => { + self.open_connectors_loading_popup(); } } self.request_redraw(); } - fn open_connectors_popup(&mut self, connectors: &[connectors::AppInfo]) { - self.bottom_pane - .show_selection_view(self.connectors_popup_params(connectors)); + fn open_connectors_loading_popup(&mut self) { + if !self.bottom_pane.replace_selection_view_if_active( + CONNECTORS_SELECTION_VIEW_ID, + self.connectors_loading_popup_params(), + ) { + self.bottom_pane + .show_selection_view(self.connectors_loading_popup_params()); + } } - fn connectors_popup_params(&self, connectors: &[connectors::AppInfo]) -> SelectionViewParams { + fn open_connectors_popup(&mut self, connectors: &[connectors::AppInfo]) { + self.bottom_pane + .show_selection_view(self.connectors_popup_params(connectors, None)); + } + + fn connectors_loading_popup_params(&self) -> SelectionViewParams { + let mut header = ColumnRenderable::new(); + header.push(Line::from("Apps".bold())); + header.push(Line::from("Loading installed and available apps...".dim())); + + SelectionViewParams { + view_id: Some(CONNECTORS_SELECTION_VIEW_ID), + header: Box::new(header), + items: vec![SelectionItem { + name: "Loading apps...".to_string(), + description: Some("This updates when the full list is ready.".to_string()), + is_disabled: true, + ..Default::default() + }], + ..Default::default() + } + } + + fn connectors_popup_params( + &self, + connectors: &[connectors::AppInfo], + selected_connector_id: Option<&str>, + ) -> SelectionViewParams { let total = connectors.len(); let installed = connectors .iter() @@ -7785,6 +7818,11 @@ impl ChatWidget { header.push(Line::from( format!("Installed {installed} of {total} available apps.").dim(), )); + let initial_selected_idx = selected_connector_id.and_then(|selected_connector_id| { + connectors + .iter() + .position(|connector| connector.id == selected_connector_id) + }); let mut items: Vec = Vec::with_capacity(connectors.len()); for connector in connectors { let connector_label = connectors::connector_display_label(connector); @@ -7853,14 +7891,28 @@ impl ChatWidget { is_searchable: true, search_placeholder: Some("Type to search apps".to_string()), col_width_mode: ColumnWidthMode::AutoAllRows, + initial_selected_idx, ..Default::default() } } fn refresh_connectors_popup_if_open(&mut self, connectors: &[connectors::AppInfo]) { + let selected_connector_id = + if let (Some(selected_index), ConnectorsCacheState::Ready(snapshot)) = ( + self.bottom_pane + .selected_index_for_active_view(CONNECTORS_SELECTION_VIEW_ID), + &self.connectors_cache, + ) { + snapshot + .connectors + .get(selected_index) + .map(|connector| connector.id.as_str()) + } else { + None + }; let _ = self.bottom_pane.replace_selection_view_if_active( CONNECTORS_SELECTION_VIEW_ID, - self.connectors_popup_params(connectors), + self.connectors_popup_params(connectors, selected_connector_id), ); } @@ -8185,15 +8237,28 @@ impl ChatWidget { } } } - self.refresh_connectors_popup_if_open(&snapshot.connectors); - if is_final || !matches!(self.connectors_cache, ConnectorsCacheState::Ready(_)) { + if is_final { + self.connectors_partial_snapshot = None; + self.refresh_connectors_popup_if_open(&snapshot.connectors); self.connectors_cache = ConnectorsCacheState::Ready(snapshot.clone()); + } else { + self.connectors_partial_snapshot = Some(snapshot.clone()); } self.bottom_pane.set_connectors_snapshot(Some(snapshot)); } Err(err) => { - if matches!(self.connectors_cache, ConnectorsCacheState::Ready(_)) { + let partial_snapshot = self.connectors_partial_snapshot.take(); + if let ConnectorsCacheState::Ready(snapshot) = &self.connectors_cache { warn!("failed to refresh apps list; retaining current apps snapshot: {err}"); + self.bottom_pane + .set_connectors_snapshot(Some(snapshot.clone())); + } else if let Some(snapshot) = partial_snapshot { + warn!( + "failed to load full apps list; falling back to installed apps snapshot: {err}" + ); + self.refresh_connectors_popup_if_open(&snapshot.connectors); + self.connectors_cache = ConnectorsCacheState::Ready(snapshot.clone()); + self.bottom_pane.set_connectors_snapshot(Some(snapshot)); } else { self.connectors_cache = ConnectorsCacheState::Failed(err); self.bottom_pane.set_connectors_snapshot(None); diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__apps_popup_loading_state.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__apps_popup_loading_state.snap new file mode 100644 index 0000000000..e75302e5c2 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__apps_popup_loading_state.snap @@ -0,0 +1,8 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: before +--- + Apps + Loading installed and available apps... + +› 1. Loading apps... This updates when the full list is ready. diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 7307891106..ef8676a109 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -1849,6 +1849,7 @@ async fn make_chatwidget_manual( agent_turn_running: false, mcp_startup_status: None, connectors_cache: ConnectorsCacheState::default(), + connectors_partial_snapshot: None, connectors_prefetch_in_flight: false, connectors_force_refetch_pending: false, interrupts: InterruptManager::new(), @@ -6370,7 +6371,7 @@ fn render_bottom_popup(chat: &ChatWidget, width: u16) -> String { } #[tokio::test] -async fn apps_popup_refreshes_when_connectors_snapshot_updates() { +async fn apps_popup_stays_loading_until_final_snapshot_updates() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; chat.config .features @@ -6408,13 +6409,10 @@ async fn apps_popup_refreshes_when_connectors_snapshot_updates() { let before = render_bottom_popup(&chat, 80); assert!( - before.contains("Installed 1 of 1 available apps."), - "expected initial apps popup snapshot, got:\n{before}" - ); - assert!( - before.contains("Installed. Press Enter to open the app page"), - "expected selected app description to explain the app page action, got:\n{before}" + before.contains("Loading installed and available apps..."), + "expected /apps to stay in the loading state until the full list arrives, got:\n{before}" ); + assert_snapshot!("apps_popup_loading_state", before); chat.on_connectors_loaded( Ok(ConnectorsSnapshot { @@ -6550,6 +6548,125 @@ async fn apps_refresh_failure_keeps_existing_full_snapshot() { ); } +#[tokio::test] +async fn apps_popup_preserves_selected_app_across_refresh() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; + chat.config + .features + .enable(Feature::Apps) + .expect("test config should allow feature update"); + chat.bottom_pane.set_connectors_enabled(true); + + chat.on_connectors_loaded( + Ok(ConnectorsSnapshot { + connectors: vec![ + codex_chatgpt::connectors::AppInfo { + id: "notion".to_string(), + name: "Notion".to_string(), + description: Some("Workspace docs".to_string()), + logo_url: None, + logo_url_dark: None, + distribution_channel: None, + branding: None, + app_metadata: None, + labels: None, + install_url: Some("https://example.test/notion".to_string()), + is_accessible: true, + is_enabled: true, + plugin_display_names: Vec::new(), + }, + codex_chatgpt::connectors::AppInfo { + id: "slack".to_string(), + name: "Slack".to_string(), + description: Some("Team chat".to_string()), + logo_url: None, + logo_url_dark: None, + distribution_channel: None, + branding: None, + app_metadata: None, + labels: None, + install_url: Some("https://example.test/slack".to_string()), + is_accessible: true, + is_enabled: true, + plugin_display_names: Vec::new(), + }, + ], + }), + true, + ); + chat.add_connectors_output(); + chat.handle_key_event(KeyEvent::from(KeyCode::Down)); + + let before = render_bottom_popup(&chat, 80); + assert!( + before.contains("› Slack"), + "expected Slack to be selected before refresh, got:\n{before}" + ); + + chat.on_connectors_loaded( + Ok(ConnectorsSnapshot { + connectors: vec![ + codex_chatgpt::connectors::AppInfo { + id: "airtable".to_string(), + name: "Airtable".to_string(), + description: Some("Spreadsheets".to_string()), + logo_url: None, + logo_url_dark: None, + distribution_channel: None, + branding: None, + app_metadata: None, + labels: None, + install_url: Some("https://example.test/airtable".to_string()), + is_accessible: true, + is_enabled: true, + plugin_display_names: Vec::new(), + }, + codex_chatgpt::connectors::AppInfo { + id: "notion".to_string(), + name: "Notion".to_string(), + description: Some("Workspace docs".to_string()), + logo_url: None, + logo_url_dark: None, + distribution_channel: None, + branding: None, + app_metadata: None, + labels: None, + install_url: Some("https://example.test/notion".to_string()), + is_accessible: true, + is_enabled: true, + plugin_display_names: Vec::new(), + }, + codex_chatgpt::connectors::AppInfo { + id: "slack".to_string(), + name: "Slack".to_string(), + description: Some("Team chat".to_string()), + logo_url: None, + logo_url_dark: None, + distribution_channel: None, + branding: None, + app_metadata: None, + labels: None, + install_url: Some("https://example.test/slack".to_string()), + is_accessible: true, + is_enabled: true, + plugin_display_names: Vec::new(), + }, + ], + }), + true, + ); + + let after = render_bottom_popup(&chat, 80); + assert!( + after.contains("› Slack"), + "expected Slack to stay selected after refresh, got:\n{after}" + ); + assert!( + !after.contains("› Notion"), + "did not expect selection to reset to Notion after refresh, got:\n{after}" + ); +} + #[tokio::test] async fn apps_refresh_failure_with_cached_snapshot_triggers_pending_force_refetch() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; @@ -6591,7 +6708,7 @@ async fn apps_refresh_failure_with_cached_snapshot_triggers_pending_force_refetc } #[tokio::test] -async fn apps_partial_refresh_uses_same_filtering_as_full_refresh() { +async fn apps_popup_keeps_existing_full_snapshot_while_partial_refresh_loads() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; chat.config .features @@ -6684,12 +6801,67 @@ async fn apps_partial_refresh_uses_same_filtering_as_full_refresh() { let popup = render_bottom_popup(&chat, 80); assert!( - popup.contains("Installed 1 of 1 available apps."), - "expected partial refresh popup to use filtered connectors, got:\n{popup}" + popup.contains("Installed 1 of 2 available apps."), + "expected popup to keep the last full snapshot while partial refresh loads, got:\n{popup}" ); assert!( !popup.contains("Hidden OpenAI"), - "expected disallowed connector to be filtered from partial refresh popup, got:\n{popup}" + "expected popup to ignore partial refresh rows until the full list arrives, got:\n{popup}" + ); +} + +#[tokio::test] +async fn apps_refresh_failure_without_full_snapshot_falls_back_to_installed_apps() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; + chat.config + .features + .enable(Feature::Apps) + .expect("test config should allow feature update"); + chat.bottom_pane.set_connectors_enabled(true); + + chat.on_connectors_loaded( + Ok(ConnectorsSnapshot { + connectors: vec![codex_chatgpt::connectors::AppInfo { + id: "unit_test_apps_refresh_failure_fallback_connector".to_string(), + name: "Notion".to_string(), + description: Some("Workspace docs".to_string()), + logo_url: None, + logo_url_dark: None, + distribution_channel: None, + branding: None, + app_metadata: None, + labels: None, + install_url: Some("https://example.test/notion".to_string()), + is_accessible: true, + is_enabled: true, + plugin_display_names: Vec::new(), + }], + }), + false, + ); + + chat.add_connectors_output(); + let loading_popup = render_bottom_popup(&chat, 80); + assert!( + loading_popup.contains("Loading installed and available apps..."), + "expected /apps to keep showing loading before the final result, got:\n{loading_popup}" + ); + + chat.on_connectors_loaded(Err("failed to load apps".to_string()), true); + + assert_matches!( + &chat.connectors_cache, + ConnectorsCacheState::Ready(snapshot) if snapshot.connectors.len() == 1 + ); + + let popup = render_bottom_popup(&chat, 80); + assert!( + popup.contains("Installed 1 of 1 available apps."), + "expected /apps to fall back to the installed apps snapshot, got:\n{popup}" + ); + assert!( + popup.contains("Installed. Press Enter to open the app page"), + "expected the fallback popup to behave like the installed apps view, got:\n{popup}" ); }