[codex] Preserve remote plugin directory order (#28395)

## Summary

- preserve the plugin directory endpoint's response order while merging
installed state
- append unmatched installed-only plugins afterward when requested
- add focused coverage for directory order and installed-only placement

## Why

The remote marketplace merge currently reconstructs plugins through
ordered maps and sets, then sorts the result alphabetically by display
name. That discards any ordering supplied by the plugin directory
endpoint before the list reaches Desktop.

## Implementation

Directory plugin IDs are unique, so the merge now iterates the directory
vector directly in response order. For each directory plugin, it removes
matching installed state from an ID-indexed map and builds the summary.
Any entries left in the installed map are installed-only plugins and are
appended when `include_installed_only` is enabled.

There is no separate rank field, rank map, or final sort. Desktop
therefore receives directory order—including any backend ranking—and can
preserve it within its existing stable UI state tiers.

## Testing

- `just test -p codex-core-plugins` (225 passed)
This commit is contained in:
jameswt-oai
2026-06-15 16:09:43 -07:00
committed by GitHub
Unverified
parent db8927a8e2
commit 4c1c9bf327
2 changed files with 99 additions and 28 deletions
+21 -28
View File
@@ -27,6 +27,10 @@ mod catalog_cache;
mod remote_installed_plugin_sync;
mod share;
#[cfg(test)]
#[path = "remote_tests.rs"]
mod tests;
pub use remote_installed_plugin_sync::RemoteInstalledPluginBundleSyncError;
pub use remote_installed_plugin_sync::RemoteInstalledPluginBundleSyncOutcome;
pub use remote_installed_plugin_sync::RemotePluginCacheMutationGuard;
@@ -812,40 +816,29 @@ fn build_remote_marketplace(
installed_plugins: Vec<RemotePluginInstalledItem>,
include_installed_only: bool,
) -> Result<Option<RemoteMarketplace>, RemotePluginCatalogError> {
let directory_plugins = directory_plugins
.into_iter()
.map(|plugin| (plugin.id.clone(), plugin))
.collect::<BTreeMap<_, _>>();
let installed_plugins = installed_plugins
let mut installed_plugins = installed_plugins
.into_iter()
.map(|plugin| (plugin.plugin.id.clone(), plugin))
.collect::<BTreeMap<_, _>>();
let plugin_ids = directory_plugins
.keys()
.chain(
include_installed_only
.then_some(&installed_plugins)
.into_iter()
.flat_map(|plugins| plugins.keys()),
)
.cloned()
.collect::<BTreeSet<_>>();
if plugin_ids.is_empty() {
let mut plugins = directory_plugins
.into_iter()
.map(|plugin| {
let installed_plugin = installed_plugins.remove(&plugin.id);
build_remote_plugin_summary(&plugin, installed_plugin.as_ref())
})
.collect::<Result<Vec<_>, _>>()?;
if include_installed_only {
plugins.extend(
installed_plugins
.into_values()
.map(|plugin| build_remote_plugin_summary(&plugin.plugin, Some(&plugin)))
.collect::<Result<Vec<_>, _>>()?,
);
}
if plugins.is_empty() {
return Ok(None);
}
let mut plugins = plugin_ids
.into_iter()
.filter_map(|plugin_id| {
let directory_plugin = directory_plugins.get(&plugin_id);
let installed_plugin = installed_plugins.get(&plugin_id);
directory_plugin
.or_else(|| installed_plugin.map(|plugin| &plugin.plugin))
.map(|plugin| (plugin, installed_plugin))
})
.map(|(plugin, installed_plugin)| build_remote_plugin_summary(plugin, installed_plugin))
.collect::<Result<Vec<_>, _>>()?;
sort_remote_plugin_summaries_by_display_name(&mut plugins);
Ok(Some(RemoteMarketplace {
name: name.to_string(),
display_name: display_name.to_string(),
+78
View File
@@ -0,0 +1,78 @@
use super::*;
use pretty_assertions::assert_eq;
#[test]
fn build_remote_marketplace_preserves_directory_order_and_appends_installed_only_plugins() {
let directory_plugins = vec![
directory_plugin("plugin-z", "zulu"),
directory_plugin("plugin-m", "mike"),
];
let installed_plugins = vec![RemotePluginInstalledItem {
plugin: directory_plugin("plugin-a", "alpha"),
enabled: true,
disabled_skill_names: Vec::new(),
}];
let marketplace = build_remote_marketplace(
"marketplace",
"Marketplace",
directory_plugins,
installed_plugins,
/*include_installed_only*/ true,
)
.expect("marketplace should be valid")
.expect("marketplace should not be empty");
assert_eq!(
marketplace
.plugins
.into_iter()
.map(|plugin| plugin.remote_plugin_id)
.collect::<Vec<_>>(),
vec!["plugin-z", "plugin-m", "plugin-a"]
);
}
fn directory_plugin(id: &str, name: &str) -> RemotePluginDirectoryItem {
RemotePluginDirectoryItem {
id: id.to_string(),
name: name.to_string(),
scope: RemotePluginScope::Global,
discoverability: None,
creator_account_user_id: None,
creator_name: None,
share_url: None,
share_principals: None,
installation_policy: PluginInstallPolicy::Available,
authentication_policy: PluginAuthPolicy::OnUse,
availability: PluginAvailability::Available,
release: RemotePluginReleaseResponse {
version: None,
display_name: name.to_string(),
description: String::new(),
bundle_download_url: None,
app_ids: Vec::new(),
app_manifest: None,
app_templates: Vec::new(),
keywords: Vec::new(),
interface: RemotePluginReleaseInterfaceResponse {
short_description: None,
long_description: None,
developer_name: None,
category: None,
capabilities: Vec::new(),
website_url: None,
privacy_policy_url: None,
terms_of_service_url: None,
brand_color: None,
default_prompt: None,
default_prompts: None,
composer_icon_url: None,
logo_url: None,
screenshot_urls: Vec::new(),
},
skills: Vec::new(),
mcp_servers: Vec::new(),
},
}
}