From 2d5a3bfe76d26c6fe3153ddd4ea5f085185e3d72 Mon Sep 17 00:00:00 2001 From: canvrno-oai Date: Tue, 24 Mar 2026 09:35:52 -0700 Subject: [PATCH] [Codex TUI] - Sort /plugins TUI menu by installed status first, alpha second (#15558) Updates plugin ordering so installed plugins are listed first, with alphabetical sorting applied within the installed and uninstalled groups. The behavior is now consistent across both `tui` and `tui_app_server`, and related tests/snapshots were updated. --- codex-rs/tui/src/chatwidget/plugins.rs | 96 ++++++++++++------- ...ts__plugins_popup_curated_marketplace.snap | 10 +- codex-rs/tui/src/chatwidget/tests.rs | 16 ++-- .../tui_app_server/src/chatwidget/plugins.rs | 96 ++++++++++++------- ...ts__plugins_popup_curated_marketplace.snap | 10 +- .../tui_app_server/src/chatwidget/tests.rs | 16 ++-- 6 files changed, 144 insertions(+), 100 deletions(-) diff --git a/codex-rs/tui/src/chatwidget/plugins.rs b/codex-rs/tui/src/chatwidget/plugins.rs index 7bf4516f4..a04cf249c 100644 --- a/codex-rs/tui/src/chatwidget/plugins.rs +++ b/codex-rs/tui/src/chatwidget/plugins.rs @@ -649,44 +649,66 @@ impl ChatWidget { )); } - let mut items: Vec = Vec::new(); - for marketplace in marketplaces { - let marketplace_label = marketplace_display_name(marketplace); - for plugin in &marketplace.plugins { - let display_name = plugin_display_name(plugin); - let status_label = plugin_status_label(plugin); - let description = plugin_brief_description(plugin, &marketplace_label); - let selected_description = - format!("{status_label}. Press Enter to view plugin details."); - let search_value = format!( - "{display_name} {} {} {}", - plugin.id, plugin.name, marketplace_label - ); - let cwd = self.config.cwd.clone(); - let plugin_display_name = display_name.clone(); - let marketplace_path = marketplace.path.clone(); - let plugin_name = plugin.name.clone(); + let mut plugin_entries: Vec<(&PluginMarketplaceEntry, &PluginSummary, String)> = + marketplaces + .iter() + .flat_map(|marketplace| { + marketplace + .plugins + .iter() + .map(move |plugin| (*marketplace, plugin, plugin_display_name(plugin))) + }) + .collect(); + plugin_entries.sort_by(|left, right| { + right + .1 + .installed + .cmp(&left.1.installed) + .then_with(|| { + left.2 + .to_ascii_lowercase() + .cmp(&right.2.to_ascii_lowercase()) + }) + .then_with(|| left.2.cmp(&right.2)) + .then_with(|| left.1.name.cmp(&right.1.name)) + .then_with(|| left.1.id.cmp(&right.1.id)) + }); - items.push(SelectionItem { - name: format!("{display_name} · {marketplace_label}"), - description: Some(description), - selected_description: Some(selected_description), - search_value: Some(search_value), - actions: vec![Box::new(move |tx| { - tx.send(AppEvent::OpenPluginDetailLoading { - plugin_display_name: plugin_display_name.clone(), - }); - tx.send(AppEvent::FetchPluginDetail { - cwd: cwd.clone(), - params: codex_app_server_protocol::PluginReadParams { - marketplace_path: marketplace_path.clone(), - plugin_name: plugin_name.clone(), - }, - }); - })], - ..Default::default() - }); - } + let mut items: Vec = Vec::new(); + for (marketplace, plugin, display_name) in plugin_entries { + let marketplace_label = marketplace_display_name(marketplace); + let status_label = plugin_status_label(plugin); + let description = plugin_brief_description(plugin, &marketplace_label); + let selected_description = + format!("{status_label}. Press Enter to view plugin details."); + let search_value = format!( + "{display_name} {} {} {}", + plugin.id, plugin.name, marketplace_label + ); + let cwd = self.config.cwd.clone(); + let plugin_display_name = display_name.clone(); + let marketplace_path = marketplace.path.clone(); + let plugin_name = plugin.name.clone(); + + items.push(SelectionItem { + name: format!("{display_name} · {marketplace_label}"), + description: Some(description), + selected_description: Some(selected_description), + search_value: Some(search_value), + actions: vec![Box::new(move |tx| { + tx.send(AppEvent::OpenPluginDetailLoading { + plugin_display_name: plugin_display_name.clone(), + }); + tx.send(AppEvent::FetchPluginDetail { + cwd: cwd.clone(), + params: codex_app_server_protocol::PluginReadParams { + marketplace_path: marketplace_path.clone(), + plugin_name: plugin_name.clone(), + }, + }); + })], + ..Default::default() + }); } if items.is_empty() { diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__plugins_popup_curated_marketplace.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__plugins_popup_curated_marketplace.snap index b85c22363..c23daeea1 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__plugins_popup_curated_marketplace.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__plugins_popup_curated_marketplace.snap @@ -9,12 +9,12 @@ expression: popup Using cached marketplace data: remote sync timed out Type to search plugins -› Bravo Search · ChatGPT Marketplace Can be installed. Press Enter to view plugin details. - Alpha Sync · ChatGPT Marketplace Installed · Disabled · ChatGPT Marketplace · Already - installed but disabled. - Starter · ChatGPT Marketplace Available by default · ChatGPT Marketplace · Included by - default. +› Alpha Sync · ChatGPT Marketplace Installed · Disabled. Press Enter to view plugin details. + Bravo Search · ChatGPT Marketplace Can be installed · ChatGPT Marketplace · Search docs and + tickets. Hidden Repo Plugin · Repo Marketplace Can be installed · Repo Marketplace · Should not be shown in /plugins. + Starter · ChatGPT Marketplace Available by default · ChatGPT Marketplace · Included by + default. Press esc to close. diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index a31b4d686..78bb4cf22 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -7239,7 +7239,7 @@ async fn plugins_popup_loading_state_snapshot() { } #[tokio::test] -async fn plugins_popup_snapshot_shows_all_marketplaces_and_preserves_response_order() { +async fn plugins_popup_snapshot_shows_all_marketplaces_and_sorts_installed_then_name() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; chat.set_feature_enabled(Feature::Plugins, true); @@ -7292,13 +7292,13 @@ async fn plugins_popup_snapshot_shows_all_marketplaces_and_preserves_response_or "expected /plugins to include non-curated marketplaces, got:\n{popup}" ); assert!( - plugins_test_popup_row_position(&popup, "Bravo Search") - < plugins_test_popup_row_position(&popup, "Alpha Sync") - && plugins_test_popup_row_position(&popup, "Alpha Sync") - < plugins_test_popup_row_position(&popup, "Starter") - && plugins_test_popup_row_position(&popup, "Starter") - < plugins_test_popup_row_position(&popup, "Hidden Repo Plugin"), - "expected /plugins rows to keep response order, got:\n{popup}" + plugins_test_popup_row_position(&popup, "Alpha Sync") + < plugins_test_popup_row_position(&popup, "Bravo Search") + && plugins_test_popup_row_position(&popup, "Bravo Search") + < plugins_test_popup_row_position(&popup, "Hidden Repo Plugin") + && plugins_test_popup_row_position(&popup, "Hidden Repo Plugin") + < plugins_test_popup_row_position(&popup, "Starter"), + "expected /plugins rows to sort installed plugins first, then alphabetically, got:\n{popup}" ); } diff --git a/codex-rs/tui_app_server/src/chatwidget/plugins.rs b/codex-rs/tui_app_server/src/chatwidget/plugins.rs index 7bf4516f4..a04cf249c 100644 --- a/codex-rs/tui_app_server/src/chatwidget/plugins.rs +++ b/codex-rs/tui_app_server/src/chatwidget/plugins.rs @@ -649,44 +649,66 @@ impl ChatWidget { )); } - let mut items: Vec = Vec::new(); - for marketplace in marketplaces { - let marketplace_label = marketplace_display_name(marketplace); - for plugin in &marketplace.plugins { - let display_name = plugin_display_name(plugin); - let status_label = plugin_status_label(plugin); - let description = plugin_brief_description(plugin, &marketplace_label); - let selected_description = - format!("{status_label}. Press Enter to view plugin details."); - let search_value = format!( - "{display_name} {} {} {}", - plugin.id, plugin.name, marketplace_label - ); - let cwd = self.config.cwd.clone(); - let plugin_display_name = display_name.clone(); - let marketplace_path = marketplace.path.clone(); - let plugin_name = plugin.name.clone(); + let mut plugin_entries: Vec<(&PluginMarketplaceEntry, &PluginSummary, String)> = + marketplaces + .iter() + .flat_map(|marketplace| { + marketplace + .plugins + .iter() + .map(move |plugin| (*marketplace, plugin, plugin_display_name(plugin))) + }) + .collect(); + plugin_entries.sort_by(|left, right| { + right + .1 + .installed + .cmp(&left.1.installed) + .then_with(|| { + left.2 + .to_ascii_lowercase() + .cmp(&right.2.to_ascii_lowercase()) + }) + .then_with(|| left.2.cmp(&right.2)) + .then_with(|| left.1.name.cmp(&right.1.name)) + .then_with(|| left.1.id.cmp(&right.1.id)) + }); - items.push(SelectionItem { - name: format!("{display_name} · {marketplace_label}"), - description: Some(description), - selected_description: Some(selected_description), - search_value: Some(search_value), - actions: vec![Box::new(move |tx| { - tx.send(AppEvent::OpenPluginDetailLoading { - plugin_display_name: plugin_display_name.clone(), - }); - tx.send(AppEvent::FetchPluginDetail { - cwd: cwd.clone(), - params: codex_app_server_protocol::PluginReadParams { - marketplace_path: marketplace_path.clone(), - plugin_name: plugin_name.clone(), - }, - }); - })], - ..Default::default() - }); - } + let mut items: Vec = Vec::new(); + for (marketplace, plugin, display_name) in plugin_entries { + let marketplace_label = marketplace_display_name(marketplace); + let status_label = plugin_status_label(plugin); + let description = plugin_brief_description(plugin, &marketplace_label); + let selected_description = + format!("{status_label}. Press Enter to view plugin details."); + let search_value = format!( + "{display_name} {} {} {}", + plugin.id, plugin.name, marketplace_label + ); + let cwd = self.config.cwd.clone(); + let plugin_display_name = display_name.clone(); + let marketplace_path = marketplace.path.clone(); + let plugin_name = plugin.name.clone(); + + items.push(SelectionItem { + name: format!("{display_name} · {marketplace_label}"), + description: Some(description), + selected_description: Some(selected_description), + search_value: Some(search_value), + actions: vec![Box::new(move |tx| { + tx.send(AppEvent::OpenPluginDetailLoading { + plugin_display_name: plugin_display_name.clone(), + }); + tx.send(AppEvent::FetchPluginDetail { + cwd: cwd.clone(), + params: codex_app_server_protocol::PluginReadParams { + marketplace_path: marketplace_path.clone(), + plugin_name: plugin_name.clone(), + }, + }); + })], + ..Default::default() + }); } if items.is_empty() { diff --git a/codex-rs/tui_app_server/src/chatwidget/snapshots/codex_tui_app_server__chatwidget__tests__plugins_popup_curated_marketplace.snap b/codex-rs/tui_app_server/src/chatwidget/snapshots/codex_tui_app_server__chatwidget__tests__plugins_popup_curated_marketplace.snap index c191f219c..f553f9074 100644 --- a/codex-rs/tui_app_server/src/chatwidget/snapshots/codex_tui_app_server__chatwidget__tests__plugins_popup_curated_marketplace.snap +++ b/codex-rs/tui_app_server/src/chatwidget/snapshots/codex_tui_app_server__chatwidget__tests__plugins_popup_curated_marketplace.snap @@ -8,12 +8,12 @@ expression: popup Using cached marketplace data: remote sync timed out Type to search plugins -› Bravo Search · ChatGPT Marketplace Can be installed. Press Enter to view plugin details. - Alpha Sync · ChatGPT Marketplace Installed · Disabled · ChatGPT Marketplace · Already - installed but disabled. - Starter · ChatGPT Marketplace Available by default · ChatGPT Marketplace · Included by - default. +› Alpha Sync · ChatGPT Marketplace Installed · Disabled. Press Enter to view plugin details. + Bravo Search · ChatGPT Marketplace Can be installed · ChatGPT Marketplace · Search docs and + tickets. Hidden Repo Plugin · Repo Marketplace Can be installed · Repo Marketplace · Should not be shown in /plugins. + Starter · ChatGPT Marketplace Available by default · ChatGPT Marketplace · Included by + default. Press esc to close. diff --git a/codex-rs/tui_app_server/src/chatwidget/tests.rs b/codex-rs/tui_app_server/src/chatwidget/tests.rs index 8a383f210..3e73b4455 100644 --- a/codex-rs/tui_app_server/src/chatwidget/tests.rs +++ b/codex-rs/tui_app_server/src/chatwidget/tests.rs @@ -7836,7 +7836,7 @@ async fn plugins_popup_loading_state_snapshot() { } #[tokio::test] -async fn plugins_popup_snapshot_shows_all_marketplaces_and_preserves_response_order() { +async fn plugins_popup_snapshot_shows_all_marketplaces_and_sorts_installed_then_name() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; chat.set_feature_enabled(Feature::Plugins, true); @@ -7889,13 +7889,13 @@ async fn plugins_popup_snapshot_shows_all_marketplaces_and_preserves_response_or "expected /plugins to include non-curated marketplaces, got:\n{popup}" ); assert!( - plugins_test_popup_row_position(&popup, "Bravo Search") - < plugins_test_popup_row_position(&popup, "Alpha Sync") - && plugins_test_popup_row_position(&popup, "Alpha Sync") - < plugins_test_popup_row_position(&popup, "Starter") - && plugins_test_popup_row_position(&popup, "Starter") - < plugins_test_popup_row_position(&popup, "Hidden Repo Plugin"), - "expected /plugins rows to keep response order, got:\n{popup}" + plugins_test_popup_row_position(&popup, "Alpha Sync") + < plugins_test_popup_row_position(&popup, "Bravo Search") + && plugins_test_popup_row_position(&popup, "Bravo Search") + < plugins_test_popup_row_position(&popup, "Hidden Repo Plugin") + && plugins_test_popup_row_position(&popup, "Hidden Repo Plugin") + < plugins_test_popup_row_position(&popup, "Starter"), + "expected /plugins rows to sort installed plugins first, then alphabetically, got:\n{popup}" ); }