From 64d8f387f93797b1929498592c38032c8d30b3a6 Mon Sep 17 00:00:00 2001 From: Eric Ning Date: Wed, 13 May 2026 16:59:22 -0700 Subject: [PATCH] Remove connector_openai prefix filtering (#22555) Remove unnecessary prefix filtering from codex ## Test Plan Test local cli build + make sure backend returns appropriate apps ``` cd ~/code/codex/codex-rs cargo build -p codex-cli --bin codex ./target/debug/codex ``` Appropriate apps show up in my list --- .../codex-mcp/src/connection_manager_tests.rs | 4 +- codex-rs/connectors/src/filter.rs | 165 +++++++++++- codex-rs/connectors/src/merge.rs | 87 ++++++ codex-rs/core/src/connectors_tests.rs | 252 +++--------------- codex-rs/utils/plugins/src/mcp_connector.rs | 9 +- 5 files changed, 284 insertions(+), 233 deletions(-) diff --git a/codex-rs/codex-mcp/src/connection_manager_tests.rs b/codex-rs/codex-mcp/src/connection_manager_tests.rs index 5d04dc2a3..976bd4c76 100644 --- a/codex-rs/codex-mcp/src/connection_manager_tests.rs +++ b/codex-rs/codex-mcp/src/connection_manager_tests.rs @@ -596,8 +596,8 @@ fn codex_apps_tools_cache_filters_disallowed_connectors() { create_test_tool_with_connector( CODEX_APPS_MCP_SERVER_NAME, "blocked_tool", - "connector_openai_hidden", - Some("Hidden"), + "connector_2b0a9009c9c64bf9933a3dae3f2b1254", + Some("Blocked"), ), create_test_tool_with_connector( CODEX_APPS_MCP_SERVER_NAME, diff --git a/codex-rs/connectors/src/filter.rs b/codex-rs/connectors/src/filter.rs index 82c334f82..e26291794 100644 --- a/codex-rs/connectors/src/filter.rs +++ b/codex-rs/connectors/src/filter.rs @@ -37,7 +37,6 @@ const DISALLOWED_CONNECTOR_IDS: &[&str] = &[ ]; const FIRST_PARTY_CHAT_DISALLOWED_CONNECTOR_IDS: &[&str] = &["connector_0f9c9d4592e54d0a9a12b3f44a1e2010"]; -const DISALLOWED_CONNECTOR_PREFIX: &str = "connector_openai_"; pub fn filter_disallowed_connectors( connectors: Vec, @@ -63,6 +62,166 @@ fn is_connector_id_allowed(connector_id: &str, first_party_chat_originator: bool DISALLOWED_CONNECTOR_IDS }; - !connector_id.starts_with(DISALLOWED_CONNECTOR_PREFIX) - && !disallowed_connector_ids.contains(&connector_id) + !disallowed_connector_ids.contains(&connector_id) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::metadata::connector_install_url; + use pretty_assertions::assert_eq; + + fn app(id: &str) -> AppInfo { + AppInfo { + id: id.to_string(), + name: id.to_string(), + description: None, + logo_url: None, + logo_url_dark: None, + distribution_channel: None, + install_url: None, + branding: None, + app_metadata: None, + labels: None, + is_accessible: false, + is_enabled: true, + plugin_display_names: Vec::new(), + } + } + + fn named_app(id: &str, name: &str) -> AppInfo { + AppInfo { + id: id.to_string(), + name: name.to_string(), + install_url: Some(connector_install_url(name, id)), + ..app(id) + } + } + + #[test] + fn filter_disallowed_connectors_allows_non_disallowed_connectors() { + let filtered = + filter_disallowed_connectors(vec![app("asdk_app_hidden"), app("alpha")], "codex_cli"); + assert_eq!(filtered, vec![app("asdk_app_hidden"), app("alpha")]); + } + + #[test] + fn filter_disallowed_connectors_allows_openai_prefix() { + let filtered = filter_disallowed_connectors( + vec![ + app("connector_openai_foo"), + app("connector_openai_bar"), + app("gamma"), + ], + "codex_cli", + ); + assert_eq!( + filtered, + vec![ + app("connector_openai_foo"), + app("connector_openai_bar"), + app("gamma") + ] + ); + } + + #[test] + fn filter_disallowed_connectors_filters_disallowed_connector_ids() { + let filtered = filter_disallowed_connectors( + vec![ + app("asdk_app_6938a94a61d881918ef32cb999ff937c"), + app("connector_3f8d1a79f27c4c7ba1a897ab13bf37dc"), + app("delta"), + ], + "codex_cli", + ); + assert_eq!(filtered, vec![app("delta")]); + } + + #[test] + fn first_party_chat_originator_filters_target_connector_ids() { + let filtered = filter_disallowed_connectors( + vec![ + app("connector_openai_foo"), + app("asdk_app_6938a94a61d881918ef32cb999ff937c"), + app("connector_0f9c9d4592e54d0a9a12b3f44a1e2010"), + ], + "codex_atlas", + ); + assert_eq!( + filtered, + vec![ + app("connector_openai_foo"), + app("asdk_app_6938a94a61d881918ef32cb999ff937c") + ] + ); + } + + #[test] + fn filter_tool_suggest_discoverable_connectors_keeps_only_plugin_backed_uninstalled_apps() { + let filtered = filter_tool_suggest_discoverable_connectors( + vec![ + named_app( + "connector_2128aebfecb84f64a069897515042a44", + "Google Calendar", + ), + named_app("connector_68df038e0ba48191908c8434991bbac2", "Gmail"), + named_app("connector_other", "Other"), + ], + &[AppInfo { + is_accessible: true, + ..named_app( + "connector_2128aebfecb84f64a069897515042a44", + "Google Calendar", + ) + }], + &HashSet::from([ + "connector_2128aebfecb84f64a069897515042a44".to_string(), + "connector_68df038e0ba48191908c8434991bbac2".to_string(), + ]), + "codex_cli", + ); + + assert_eq!( + filtered, + vec![named_app( + "connector_68df038e0ba48191908c8434991bbac2", + "Gmail", + )] + ); + } + + #[test] + fn filter_tool_suggest_discoverable_connectors_excludes_accessible_apps_even_when_disabled() { + let filtered = filter_tool_suggest_discoverable_connectors( + vec![ + named_app( + "connector_2128aebfecb84f64a069897515042a44", + "Google Calendar", + ), + named_app("connector_68df038e0ba48191908c8434991bbac2", "Gmail"), + ], + &[ + AppInfo { + is_accessible: true, + ..named_app( + "connector_2128aebfecb84f64a069897515042a44", + "Google Calendar", + ) + }, + AppInfo { + is_accessible: true, + is_enabled: false, + ..named_app("connector_68df038e0ba48191908c8434991bbac2", "Gmail") + }, + ], + &HashSet::from([ + "connector_2128aebfecb84f64a069897515042a44".to_string(), + "connector_68df038e0ba48191908c8434991bbac2".to_string(), + ]), + "codex_cli", + ); + + assert_eq!(filtered, Vec::::new()); + } } diff --git a/codex-rs/connectors/src/merge.rs b/codex-rs/connectors/src/merge.rs index b41ee63ad..a8bee1873 100644 --- a/codex-rs/connectors/src/merge.rs +++ b/codex-rs/connectors/src/merge.rs @@ -117,3 +117,90 @@ pub fn plugin_connector_to_app_info(connector_id: String) -> AppInfo { plugin_display_names: Vec::new(), } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::metadata::connector_install_url; + use crate::metadata::connector_mention_slug; + use pretty_assertions::assert_eq; + + fn plugin_names(names: &[&str]) -> Vec { + names.iter().map(ToString::to_string).collect() + } + + fn google_calendar_accessible_connector(plugin_display_names: &[&str]) -> AppInfo { + AppInfo { + id: "calendar".to_string(), + name: "Google Calendar".to_string(), + description: Some("Plan events".to_string()), + logo_url: Some("https://example.com/logo.png".to_string()), + logo_url_dark: Some("https://example.com/logo-dark.png".to_string()), + distribution_channel: Some("workspace".to_string()), + branding: None, + app_metadata: None, + labels: None, + install_url: None, + is_accessible: true, + is_enabled: true, + plugin_display_names: plugin_names(plugin_display_names), + } + } + + #[test] + fn merge_connectors_replaces_plugin_placeholder_name_with_accessible_name() { + let plugin = plugin_connector_to_app_info("calendar".to_string()); + let accessible = google_calendar_accessible_connector(&[]); + + let merged = merge_connectors(vec![plugin], vec![accessible]); + + assert_eq!( + merged, + vec![AppInfo { + id: "calendar".to_string(), + name: "Google Calendar".to_string(), + description: Some("Plan events".to_string()), + logo_url: Some("https://example.com/logo.png".to_string()), + logo_url_dark: Some("https://example.com/logo-dark.png".to_string()), + distribution_channel: Some("workspace".to_string()), + branding: None, + app_metadata: None, + labels: None, + install_url: Some(connector_install_url("calendar", "calendar")), + is_accessible: true, + is_enabled: true, + plugin_display_names: Vec::new(), + }] + ); + assert_eq!(connector_mention_slug(&merged[0]), "google-calendar"); + } + + #[test] + fn merge_connectors_unions_and_dedupes_plugin_display_names() { + let mut plugin = plugin_connector_to_app_info("calendar".to_string()); + plugin.plugin_display_names = plugin_names(&["sample", "alpha", "sample"]); + + let accessible = google_calendar_accessible_connector(&["beta", "alpha"]); + + let merged = merge_connectors(vec![plugin], vec![accessible]); + + assert_eq!( + merged, + vec![AppInfo { + id: "calendar".to_string(), + name: "Google Calendar".to_string(), + description: Some("Plan events".to_string()), + logo_url: Some("https://example.com/logo.png".to_string()), + logo_url_dark: Some("https://example.com/logo-dark.png".to_string()), + distribution_channel: Some("workspace".to_string()), + branding: None, + app_metadata: None, + labels: None, + install_url: Some(connector_install_url("calendar", "calendar")), + is_accessible: true, + is_enabled: true, + plugin_display_names: plugin_names(&["alpha", "beta", "sample"]), + }] + ); + } +} diff --git a/codex-rs/core/src/connectors_tests.rs b/codex-rs/core/src/connectors_tests.rs index c5798d7c1..cce249167 100644 --- a/codex-rs/core/src/connectors_tests.rs +++ b/codex-rs/core/src/connectors_tests.rs @@ -13,12 +13,8 @@ use codex_config::types::AppConfig; use codex_config::types::AppToolConfig; use codex_config::types::AppToolsConfig; use codex_config::types::AppsDefaultConfig; -use codex_connectors::filter::filter_disallowed_connectors; -use codex_connectors::filter::filter_tool_suggest_discoverable_connectors; -use codex_connectors::merge::merge_connectors; use codex_connectors::merge::plugin_connector_to_app_info; use codex_connectors::metadata::connector_install_url; -use codex_connectors::metadata::connector_mention_slug; use codex_connectors::metadata::sanitize_name; use codex_features::Feature; use codex_login::CodexAuth; @@ -62,15 +58,6 @@ fn app(id: &str) -> AppInfo { } } -fn named_app(id: &str, name: &str) -> AppInfo { - AppInfo { - id: id.to_string(), - name: name.to_string(), - install_url: Some(connector_install_url(name, id)), - ..app(id) - } -} - fn plugin_names(names: &[&str]) -> Vec { names.iter().map(ToString::to_string).collect() } @@ -89,24 +76,6 @@ fn test_tool_definition(tool_name: &str) -> Tool { } } -fn google_calendar_accessible_connector(plugin_display_names: &[&str]) -> AppInfo { - AppInfo { - id: "calendar".to_string(), - name: "Google Calendar".to_string(), - description: Some("Plan events".to_string()), - logo_url: Some("https://example.com/logo.png".to_string()), - logo_url_dark: Some("https://example.com/logo-dark.png".to_string()), - distribution_channel: Some("workspace".to_string()), - branding: None, - app_metadata: None, - labels: None, - install_url: None, - is_accessible: true, - is_enabled: true, - plugin_display_names: plugin_names(plugin_display_names), - } -} - fn codex_app_tool( tool_name: &str, connector_id: &str, @@ -147,34 +116,6 @@ fn with_accessible_connectors_cache_cleared(f: impl FnOnce() -> R) -> R { result } -#[test] -fn merge_connectors_replaces_plugin_placeholder_name_with_accessible_name() { - let plugin = plugin_connector_to_app_info("calendar".to_string()); - let accessible = google_calendar_accessible_connector(&[]); - - let merged = merge_connectors(vec![plugin], vec![accessible]); - - assert_eq!( - merged, - vec![AppInfo { - id: "calendar".to_string(), - name: "Google Calendar".to_string(), - description: Some("Plan events".to_string()), - logo_url: Some("https://example.com/logo.png".to_string()), - logo_url_dark: Some("https://example.com/logo-dark.png".to_string()), - distribution_channel: Some("workspace".to_string()), - branding: None, - app_metadata: None, - labels: None, - install_url: Some(connector_install_url("calendar", "calendar")), - is_accessible: true, - is_enabled: true, - plugin_display_names: Vec::new(), - }] - ); - assert_eq!(connector_mention_slug(&merged[0]), "google-calendar"); -} - #[test] fn accessible_connectors_from_mcp_tools_carries_plugin_display_names() { let tools = vec![ @@ -258,50 +199,38 @@ async fn refresh_accessible_connectors_cache_from_mcp_tools_writes_latest_instal assert_eq!( cached, - vec![AppInfo { - id: "calendar".to_string(), - name: "Google Calendar".to_string(), - description: None, - logo_url: None, - logo_url_dark: None, - distribution_channel: None, - install_url: Some(connector_install_url("Google Calendar", "calendar")), - branding: None, - app_metadata: None, - labels: None, - is_accessible: true, - is_enabled: true, - plugin_display_names: plugin_names(&["calendar-plugin"]), - }] - ); -} - -#[test] -fn merge_connectors_unions_and_dedupes_plugin_display_names() { - let mut plugin = plugin_connector_to_app_info("calendar".to_string()); - plugin.plugin_display_names = plugin_names(&["sample", "alpha", "sample"]); - - let accessible = google_calendar_accessible_connector(&["beta", "alpha"]); - - let merged = merge_connectors(vec![plugin], vec![accessible]); - - assert_eq!( - merged, - vec![AppInfo { - id: "calendar".to_string(), - name: "Google Calendar".to_string(), - description: Some("Plan events".to_string()), - logo_url: Some("https://example.com/logo.png".to_string()), - logo_url_dark: Some("https://example.com/logo-dark.png".to_string()), - distribution_channel: Some("workspace".to_string()), - branding: None, - app_metadata: None, - labels: None, - install_url: Some(connector_install_url("calendar", "calendar")), - is_accessible: true, - is_enabled: true, - plugin_display_names: plugin_names(&["alpha", "beta", "sample"]), - }] + vec![ + AppInfo { + id: "calendar".to_string(), + name: "Google Calendar".to_string(), + description: None, + logo_url: None, + logo_url_dark: None, + distribution_channel: None, + install_url: Some(connector_install_url("Google Calendar", "calendar")), + branding: None, + app_metadata: None, + labels: None, + is_accessible: true, + is_enabled: true, + plugin_display_names: plugin_names(&["calendar-plugin"]), + }, + AppInfo { + id: "connector_openai_hidden".to_string(), + name: "Hidden".to_string(), + description: None, + logo_url: None, + logo_url_dark: None, + distribution_channel: None, + install_url: Some(connector_install_url("Hidden", "connector_openai_hidden")), + branding: None, + app_metadata: None, + labels: None, + is_accessible: true, + is_enabled: true, + plugin_display_names: Vec::new(), + } + ] ); } @@ -1245,55 +1174,6 @@ fn app_tool_policy_matches_prefix_stripped_tool_name_for_tool_config() { ); } -#[test] -fn filter_disallowed_connectors_allows_non_disallowed_connectors() { - let filtered = - filter_disallowed_connectors(vec![app("asdk_app_hidden"), app("alpha")], "codex_cli"); - assert_eq!(filtered, vec![app("asdk_app_hidden"), app("alpha")]); -} - -#[test] -fn filter_disallowed_connectors_filters_openai_prefix() { - let filtered = filter_disallowed_connectors( - vec![ - app("connector_openai_foo"), - app("connector_openai_bar"), - app("gamma"), - ], - "codex_cli", - ); - assert_eq!(filtered, vec![app("gamma")]); -} - -#[test] -fn filter_disallowed_connectors_filters_disallowed_connector_ids() { - let filtered = filter_disallowed_connectors( - vec![ - app("asdk_app_6938a94a61d881918ef32cb999ff937c"), - app("connector_3f8d1a79f27c4c7ba1a897ab13bf37dc"), - app("delta"), - ], - "codex_cli", - ); - assert_eq!(filtered, vec![app("delta")]); -} - -#[test] -fn first_party_chat_originator_filters_target_and_openai_prefixed_connectors() { - let filtered = filter_disallowed_connectors( - vec![ - app("connector_openai_foo"), - app("asdk_app_6938a94a61d881918ef32cb999ff937c"), - app("connector_0f9c9d4592e54d0a9a12b3f44a1e2010"), - ], - "codex_atlas", - ); - assert_eq!( - filtered, - vec![app("asdk_app_6938a94a61d881918ef32cb999ff937c")] - ); -} - #[tokio::test] async fn tool_suggest_connector_ids_include_configured_tool_suggest_discoverables() { let codex_home = tempdir().expect("tempdir should succeed"); @@ -1385,71 +1265,3 @@ discoverables = [ ))] ); } - -#[test] -fn filter_tool_suggest_discoverable_connectors_keeps_only_plugin_backed_uninstalled_apps() { - let filtered = filter_tool_suggest_discoverable_connectors( - vec![ - named_app( - "connector_2128aebfecb84f64a069897515042a44", - "Google Calendar", - ), - named_app("connector_68df038e0ba48191908c8434991bbac2", "Gmail"), - named_app("connector_other", "Other"), - ], - &[AppInfo { - is_accessible: true, - ..named_app( - "connector_2128aebfecb84f64a069897515042a44", - "Google Calendar", - ) - }], - &HashSet::from([ - "connector_2128aebfecb84f64a069897515042a44".to_string(), - "connector_68df038e0ba48191908c8434991bbac2".to_string(), - ]), - "codex_cli", - ); - - assert_eq!( - filtered, - vec![named_app( - "connector_68df038e0ba48191908c8434991bbac2", - "Gmail", - )] - ); -} - -#[test] -fn filter_tool_suggest_discoverable_connectors_excludes_accessible_apps_even_when_disabled() { - let filtered = filter_tool_suggest_discoverable_connectors( - vec![ - named_app( - "connector_2128aebfecb84f64a069897515042a44", - "Google Calendar", - ), - named_app("connector_68df038e0ba48191908c8434991bbac2", "Gmail"), - ], - &[ - AppInfo { - is_accessible: true, - ..named_app( - "connector_2128aebfecb84f64a069897515042a44", - "Google Calendar", - ) - }, - AppInfo { - is_accessible: true, - is_enabled: false, - ..named_app("connector_68df038e0ba48191908c8434991bbac2", "Gmail") - }, - ], - &HashSet::from([ - "connector_2128aebfecb84f64a069897515042a44".to_string(), - "connector_68df038e0ba48191908c8434991bbac2".to_string(), - ]), - "codex_cli", - ); - - assert_eq!(filtered, Vec::::new()); -} diff --git a/codex-rs/utils/plugins/src/mcp_connector.rs b/codex-rs/utils/plugins/src/mcp_connector.rs index 0f02258c1..0c293936c 100644 --- a/codex-rs/utils/plugins/src/mcp_connector.rs +++ b/codex-rs/utils/plugins/src/mcp_connector.rs @@ -11,8 +11,6 @@ const DISALLOWED_CONNECTOR_IDS: &[&str] = &[ ]; const FIRST_PARTY_CHAT_DISALLOWED_CONNECTOR_IDS: &[&str] = &["connector_0f9c9d4592e54d0a9a12b3f44a1e2010"]; -const ALLOWED_OPENAI_CONNECTOR_IDS: &[&str] = &["connector_openai_library"]; -const DISALLOWED_CONNECTOR_PREFIX: &str = "connector_openai_"; pub fn is_connector_id_allowed(connector_id: &str) -> bool { is_connector_id_allowed_for_originator(connector_id, originator().value.as_str()) @@ -25,12 +23,7 @@ fn is_connector_id_allowed_for_originator(connector_id: &str, originator_value: DISALLOWED_CONNECTOR_IDS }; - if ALLOWED_OPENAI_CONNECTOR_IDS.contains(&connector_id) { - return true; - } - - !connector_id.starts_with(DISALLOWED_CONNECTOR_PREFIX) - && !disallowed_connector_ids.contains(&connector_id) + !disallowed_connector_ids.contains(&connector_id) } pub fn sanitize_name(name: &str) -> String {