diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 3d55019cd..c5b165d98 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2669,6 +2669,7 @@ dependencies = [ "codex-utils-plugins", "dirs", "flate2", + "indexmap 2.13.0", "libc", "pretty_assertions", "reqwest 0.12.28", diff --git a/codex-rs/chatgpt/src/connectors.rs b/codex-rs/chatgpt/src/connectors.rs index 2e54192e8..63b34e735 100644 --- a/codex-rs/chatgpt/src/connectors.rs +++ b/codex-rs/chatgpt/src/connectors.rs @@ -1,3 +1,4 @@ +use std::collections::HashMap; use std::collections::HashSet; use std::time::Duration; @@ -154,20 +155,21 @@ pub fn connectors_for_plugin_apps( connectors: Vec, plugin_apps: &[AppConnectorId], ) -> Vec { - let plugin_app_ids = plugin_apps - .iter() - .map(|connector_id| connector_id.0.as_str()) - .collect::>(); - let connectors = merge_plugin_connectors( connectors, plugin_apps .iter() .map(|connector_id| connector_id.0.clone()), ); - filter_disallowed_connectors(connectors, originator().value.as_str()) - .into_iter() - .filter(|connector| plugin_app_ids.contains(connector.id.as_str())) + let mut connectors_by_id = + filter_disallowed_connectors(connectors, originator().value.as_str()) + .into_iter() + .map(|connector| (connector.id.clone(), connector)) + .collect::>(); + + plugin_apps + .iter() + .filter_map(|connector_id| connectors_by_id.remove(connector_id.0.as_str())) .collect() } @@ -266,13 +268,14 @@ mod tests { let connectors = connectors_for_plugin_apps( vec![app("alpha"), app("beta")], &[ + AppConnectorId("gmail".to_string()), AppConnectorId("alpha".to_string()), AppConnectorId("gmail".to_string()), ], ); assert_eq!( connectors, - vec![app("alpha"), merged_app("gmail", /*is_accessible*/ false)] + vec![merged_app("gmail", /*is_accessible*/ false), app("alpha")] ); } diff --git a/codex-rs/core-plugins/Cargo.toml b/codex-rs/core-plugins/Cargo.toml index 83df070dd..d0096e445 100644 --- a/codex-rs/core-plugins/Cargo.toml +++ b/codex-rs/core-plugins/Cargo.toml @@ -31,6 +31,7 @@ codex-utils-plugins = { workspace = true } chrono = { workspace = true } dirs = { workspace = true } flate2 = { workspace = true } +indexmap = { workspace = true, features = ["serde"] } reqwest = { workspace = true } semver = { workspace = true } serde = { workspace = true, features = ["derive"] } diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index 9e0ef7c47..121b583cb 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -32,6 +32,7 @@ use codex_protocol::protocol::Product; use codex_protocol::protocol::SkillScope; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_plugins::find_plugin_manifest_path; +use indexmap::IndexMap; use serde::Deserialize; use serde_json::Map as JsonMap; use serde_json::Value as JsonValue; @@ -99,7 +100,7 @@ impl PluginMcpFile { #[serde(rename_all = "camelCase")] struct PluginAppFile { #[serde(default)] - apps: HashMap, + apps: IndexMap, } #[derive(Debug, Default, Deserialize)] @@ -883,10 +884,7 @@ async fn load_apps_from_paths( } }; - let mut apps: Vec = parsed.apps.into_values().collect(); - apps.sort_unstable_by(|left, right| left.id.cmp(&right.id)); - - connector_ids.extend(apps.into_iter().filter_map(|app| { + connector_ids.extend(parsed.apps.into_values().filter_map(|app| { if app.id.trim().is_empty() { warn!( plugin = %plugin_root.display(), @@ -898,7 +896,8 @@ async fn load_apps_from_paths( } })); } - connector_ids.dedup(); + let mut seen_connector_ids = HashSet::new(); + connector_ids.retain(|connector_id| seen_connector_ids.insert(connector_id.0.clone())); connector_ids } diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index 0131ef22e..f9fae5631 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -985,6 +985,50 @@ async fn effective_apps_dedupes_connector_ids_across_plugins() { ); } +#[tokio::test] +async fn effective_apps_preserves_app_config_order() { + let codex_home = TempDir::new().unwrap(); + let plugin_root = codex_home + .path() + .join("plugins/cache") + .join("test/sample/local"); + + write_file( + &plugin_root.join(".codex-plugin/plugin.json"), + r#"{"name":"sample"}"#, + ); + write_file( + &plugin_root.join(".app.json"), + r#"{ + "apps": { + "slack": { + "id": "connector_slack" + }, + "github": { + "id": "connector_github" + }, + "slack-copy": { + "id": "connector_slack" + } + } +}"#, + ); + + let outcome = load_plugins_from_config( + &plugin_config_toml(/*enabled*/ true, /*plugins_feature_enabled*/ true), + codex_home.path(), + ) + .await; + + assert_eq!( + outcome.effective_apps(), + vec![ + AppConnectorId("connector_slack".to_string()), + AppConnectorId("connector_github".to_string()), + ] + ); +} + #[test] fn capability_index_filters_inactive_and_zero_capability_plugins() { let codex_home = TempDir::new().unwrap();