diff --git a/codex-rs/app-server/src/request_processors/plugins.rs b/codex-rs/app-server/src/request_processors/plugins.rs index cdc9bdd74..3ca87d036 100644 --- a/codex-rs/app-server/src/request_processors/plugins.rs +++ b/codex-rs/app-server/src/request_processors/plugins.rs @@ -1442,7 +1442,9 @@ impl PluginRequestProcessor { .await; } - let plugin_apps = load_plugin_apps(result.installed_path.as_path()).await; + let plugin_app_declarations = load_plugin_apps(result.installed_path.as_path()).await; + let plugin_apps = + codex_plugin::app_connector_ids_from_declarations(&plugin_app_declarations); let auth = self.auth_manager.auth().await; let apps_needing_auth = self .plugin_apps_needing_auth_for_install( @@ -1559,52 +1561,55 @@ impl PluginRequestProcessor { } let is_chatgpt_auth = auth.as_ref().is_some_and(CodexAuth::is_chatgpt_auth); - let apps_needing_auth = - if let Some(app_ids_needing_auth) = install_result.app_ids_needing_auth { - if app_ids_needing_auth.is_empty() - || !config.features.apps_enabled_for_auth(is_chatgpt_auth) - { - Vec::new() - } else { - let plugin_apps = app_ids_needing_auth - .into_iter() - .map(codex_plugin::AppConnectorId) - .collect::>(); - let app_category_by_id = remote_detail - .app_manifest - .as_ref() - .map(plugin_app_category_by_id_from_value) - .unwrap_or_default(); - let all_connectors = connectors::list_cached_all_connectors(&config) - .await - .unwrap_or_default(); - connectors::connectors_for_plugin_apps(all_connectors, &plugin_apps) - .into_iter() - .map(|connector| { - let category = app_category_by_id - .get(&connector.id) - .cloned() - .or_else(|| connector.category()); - AppSummary { - category, - id: connector.id, - name: connector.name, - description: connector.description, - install_url: connector.install_url, - } - }) - .collect() - } + let apps_needing_auth = if let Some(app_ids_needing_auth) = + install_result.app_ids_needing_auth + { + if app_ids_needing_auth.is_empty() + || !config.features.apps_enabled_for_auth(is_chatgpt_auth) + { + Vec::new() } else { - let plugin_apps = load_plugin_apps(result.installed_path.as_path()).await; - self.plugin_apps_needing_auth_for_install( - &config, - is_chatgpt_auth, - &result.plugin_id.as_key(), - &plugin_apps, - ) - .await - }; + let plugin_apps = app_ids_needing_auth + .into_iter() + .map(codex_plugin::AppConnectorId) + .collect::>(); + let app_category_by_id = remote_detail + .app_manifest + .as_ref() + .map(plugin_app_category_by_id_from_value) + .unwrap_or_default(); + let all_connectors = connectors::list_cached_all_connectors(&config) + .await + .unwrap_or_default(); + connectors::connectors_for_plugin_apps(all_connectors, &plugin_apps) + .into_iter() + .map(|connector| { + let category = app_category_by_id + .get(&connector.id) + .cloned() + .or_else(|| connector.category()); + AppSummary { + category, + id: connector.id, + name: connector.name, + description: connector.description, + install_url: connector.install_url, + } + }) + .collect() + } + } else { + let plugin_app_declarations = load_plugin_apps(result.installed_path.as_path()).await; + let plugin_apps = + codex_plugin::app_connector_ids_from_declarations(&plugin_app_declarations); + self.plugin_apps_needing_auth_for_install( + &config, + is_chatgpt_auth, + &result.plugin_id.as_key(), + &plugin_apps, + ) + .await + }; Ok(PluginInstallResponse { auth_policy: remote_detail.summary.auth_policy, @@ -1937,9 +1942,9 @@ async fn load_plugin_app_summaries( } fn plugin_app_category_by_id_from_value(value: &serde_json::Value) -> HashMap { - codex_core_plugins::loader::plugin_app_metadata_from_value(value) + codex_core_plugins::loader::plugin_app_declarations_from_value(value) .into_iter() - .filter_map(|app| app.category.map(|category| (app.id.0, category))) + .filter_map(|app| app.category.map(|category| (app.connector_id.0, category))) .collect() } diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index a89b5e59a..ef447e9c1 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -25,6 +25,7 @@ use codex_exec_server::LOCAL_FS; use codex_mcp::PluginMcpServerPlacement; use codex_mcp::parse_plugin_mcp_config; use codex_plugin::AppConnectorId; +use codex_plugin::AppDeclaration; use codex_plugin::LoadedPlugin; use codex_plugin::PluginCapabilitySummary; use codex_plugin::PluginHookSource; @@ -32,6 +33,7 @@ use codex_plugin::PluginId; use codex_plugin::PluginIdError; use codex_plugin::PluginLoadOutcome; use codex_plugin::PluginTelemetryMetadata; +use codex_plugin::app_connector_ids_from_declarations; use codex_protocol::protocol::Product; use codex_protocol::protocol::SkillScope; use codex_utils_absolute_path::AbsolutePathBuf; @@ -63,12 +65,6 @@ pub struct PluginHookLoadOutcome { pub hook_load_warnings: Vec, } -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct PluginAppMetadata { - pub id: AppConnectorId, - pub category: Option, -} - enum PluginLoadScope<'a> { AllCapabilities { restriction_product: Option, @@ -835,15 +831,7 @@ fn default_mcp_config_paths(plugin_root: &Path) -> Vec { paths } -pub async fn load_plugin_apps(plugin_root: &Path) -> Vec { - load_plugin_app_metadata(plugin_root) - .await - .into_iter() - .map(|app| app.id) - .collect() -} - -pub async fn load_plugin_app_metadata(plugin_root: &Path) -> Vec { +pub async fn load_plugin_apps(plugin_root: &Path) -> Vec { if let Some(manifest) = load_plugin_manifest(plugin_root) { return load_apps_from_paths( plugin_root, @@ -854,13 +842,13 @@ pub async fn load_plugin_app_metadata(plugin_root: &Path) -> Vec Vec { +pub fn plugin_app_declarations_from_value(value: &JsonValue) -> Vec { let Ok(parsed) = serde_json::from_value::(value.clone()) else { return Vec::new(); }; - let mut apps = plugin_app_metadata_from_file(parsed, /*plugin_root*/ None); + let mut apps = app_declarations_from_file(parsed, /*plugin_root*/ None); let mut seen_connector_ids = HashSet::new(); - apps.retain(|app| seen_connector_ids.insert(app.id.0.clone())); + apps.retain(|app| seen_connector_ids.insert(app.connector_id.0.clone())); apps } @@ -1000,8 +988,8 @@ fn append_plugin_hook_file( async fn load_apps_from_paths( plugin_root: &Path, app_config_paths: Vec, -) -> Vec { - let mut apps = Vec::new(); +) -> Vec { + let mut app_declarations = Vec::new(); for app_config_path in app_config_paths { let Ok(contents) = tokio::fs::read_to_string(app_config_path.as_path()).await else { continue; @@ -1017,21 +1005,19 @@ async fn load_apps_from_paths( } }; - apps.extend(plugin_app_metadata_from_file(parsed, Some(plugin_root))); + app_declarations.extend(app_declarations_from_file(parsed, Some(plugin_root))); } - let mut seen_connector_ids = HashSet::new(); - apps.retain(|app| seen_connector_ids.insert(app.id.0.clone())); - apps + app_declarations } -fn plugin_app_metadata_from_file( +fn app_declarations_from_file( parsed: PluginAppFile, plugin_root: Option<&Path>, -) -> Vec { +) -> Vec { parsed .apps - .into_values() - .filter_map(|app| { + .into_iter() + .filter_map(|(name, app)| { if app.id.trim().is_empty() { if let Some(plugin_root) = plugin_root { warn!( @@ -1041,18 +1027,22 @@ fn plugin_app_metadata_from_file( } None } else { - Some(PluginAppMetadata { - id: AppConnectorId(app.id), - category: app - .category - .map(|category| category.trim().to_string()) - .filter(|category| !category.is_empty()), + Some(AppDeclaration { + name, + connector_id: AppConnectorId(app.id), + category: cleaned_app_category(app.category), }) } }) .collect() } +fn cleaned_app_category(category: Option) -> Option { + category + .map(|category| category.trim().to_string()) + .filter(|category| !category.is_empty()) +} + pub async fn plugin_telemetry_metadata_from_root( plugin_id: &PluginId, plugin_root: &AbsolutePathBuf, @@ -1075,6 +1065,13 @@ pub async fn plugin_telemetry_metadata_from_root( mcp_server_names.sort_unstable(); mcp_server_names.dedup(); + let app_declarations = load_apps_from_paths( + plugin_root.as_path(), + plugin_app_config_paths(plugin_root.as_path(), manifest_paths), + ) + .await; + let app_connector_ids = app_connector_ids_from_declarations(&app_declarations); + PluginTelemetryMetadata { plugin_id: plugin_id.clone(), remote_plugin_id: None, @@ -1084,14 +1081,7 @@ pub async fn plugin_telemetry_metadata_from_root( description: None, has_skills, mcp_server_names, - app_connector_ids: load_apps_from_paths( - plugin_root.as_path(), - plugin_app_config_paths(plugin_root.as_path(), manifest_paths), - ) - .await - .into_iter() - .map(|app| app.id) - .collect(), + app_connector_ids, }), } } diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index 5ffdb5376..7ac0fd070 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -5,7 +5,7 @@ use crate::loader::PluginHookLoadOutcome; use crate::loader::configured_curated_plugin_ids_from_codex_home; use crate::loader::curated_plugin_cache_version; use crate::loader::installed_plugin_telemetry_metadata; -use crate::loader::load_plugin_app_metadata; +use crate::loader::load_plugin_apps; use crate::loader::load_plugin_hooks; use crate::loader::load_plugin_hooks_from_layer_stack; use crate::loader::load_plugin_mcp_servers; @@ -63,6 +63,7 @@ use codex_plugin::AppConnectorId; use codex_plugin::PluginCapabilitySummary; use codex_plugin::PluginId; use codex_plugin::PluginIdError; +use codex_plugin::app_connector_ids_from_declarations; use codex_plugin::prompt_safe_plugin_description; use codex_protocol::protocol::HookEventName; use codex_protocol::protocol::Product; @@ -214,7 +215,14 @@ fn project_plugin_load_outcome_for_auth( for plugin in &mut plugins { if apps_route_available { if plugin.is_active() && !plugin.apps.is_empty() { - plugin.mcp_servers.clear(); + let app_declaration_names = plugin + .apps + .iter() + .map(|app| app.name.as_str()) + .collect::>(); + plugin + .mcp_servers + .retain(|name, _| !app_declaration_names.contains(name.as_str())); } } else { plugin.apps.clear(); @@ -1311,12 +1319,17 @@ impl PluginsManager { event_name: hook.event_name, }) .collect(); - let app_metadata = load_plugin_app_metadata(source_path.as_path()).await; - let apps = app_metadata.iter().map(|app| app.id.clone()).collect(); - let app_category_by_id = app_metadata - .into_iter() - .filter_map(|app| app.category.map(|category| (app.id.0, category))) - .collect(); + let app_declarations = load_plugin_apps(source_path.as_path()).await; + let apps = app_connector_ids_from_declarations(&app_declarations); + let mut seen_app_connector_ids = HashSet::new(); + let mut app_category_by_id = HashMap::new(); + for app in &app_declarations { + if seen_app_connector_ids.insert(app.connector_id.0.as_str()) + && let Some(category) = &app.category + { + app_category_by_id.insert(app.connector_id.0.clone(), category.clone()); + } + } let mut mcp_server_names = load_plugin_mcp_servers(source_path.as_path()) .await .into_keys() diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index 8df52c94d..78f7d3e08 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -30,6 +30,7 @@ use codex_config::McpServerOAuthConfig; use codex_config::McpServerToolConfig; use codex_config::types::McpServerTransportConfig; use codex_login::CodexAuth; +use codex_plugin::AppDeclaration; use codex_protocol::protocol::HookEventName; use codex_protocol::protocol::Product; use codex_utils_absolute_path::test_support::PathBufExt; @@ -94,10 +95,27 @@ fn write_auth_projection_plugin(codex_home: &Path, name: &str, include_app: bool ), ); if include_app { - write_file( - &plugin_root.join(".app.json"), - &format!(r#"{{"apps":{{"{name}":{{"id":"connector_{name}"}}}}}}"#), - ); + write_auth_projection_app(codex_home, name, name); + } +} + +fn write_auth_projection_app(codex_home: &Path, plugin_name: &str, app_name: &str) { + let plugin_root = codex_home + .join("plugins/cache") + .join("test") + .join(plugin_name) + .join("local"); + write_file( + &plugin_root.join(".app.json"), + &format!(r#"{{"apps":{{"{app_name}":{{"id":"connector_{plugin_name}"}}}}}}"#), + ); +} + +fn app_declaration(name: &str, connector_id: &str) -> AppDeclaration { + AppDeclaration { + name: name.to_string(), + connector_id: AppConnectorId(connector_id.to_string()), + category: None, } } @@ -155,7 +173,7 @@ async fn plugin_auth_projection_hides_apps_without_chatgpt_auth() { } #[tokio::test] -async fn plugin_auth_projection_hides_dual_surface_mcp_with_chatgpt_apps_route() { +async fn plugin_auth_projection_hides_matching_mcp_with_chatgpt_apps_route() { let codex_home = TempDir::new().unwrap(); write_auth_projection_plugin(codex_home.path(), "sample", /*include_app*/ true); write_auth_projection_plugin(codex_home.path(), "docs", /*include_app*/ false); @@ -219,6 +237,124 @@ async fn plugin_auth_projection_hides_dual_surface_mcp_with_agent_identity_apps_ ); } +#[tokio::test] +async fn plugin_auth_projection_keeps_non_conflicting_mcp_with_chatgpt_apps_route() { + let codex_home = TempDir::new().unwrap(); + write_auth_projection_plugin(codex_home.path(), "sample", /*include_app*/ false); + write_auth_projection_app(codex_home.path(), "sample", "sample_app"); + write_auth_projection_plugin(codex_home.path(), "docs", /*include_app*/ false); + let config = auth_projection_config(codex_home.path()).await; + let manager = PluginsManager::new_with_options( + codex_home.path().to_path_buf(), + Some(Product::Codex), + Some(AuthMode::Chatgpt), + ); + + let outcome = manager.plugins_for_config(&config).await; + + assert_eq!( + outcome.effective_apps(), + vec![AppConnectorId("connector_sample".to_string())] + ); + assert_eq!( + sorted_effective_mcp_server_names(&outcome), + vec!["docs".to_string(), "sample".to_string()] + ); + let sample = outcome + .capability_summaries() + .iter() + .find(|plugin| plugin.config_name == "sample@test") + .expect("sample plugin summary should exist"); + assert_eq!(sample.mcp_server_names, vec!["sample".to_string()]); + assert_eq!( + sample.app_connector_ids, + vec![AppConnectorId("connector_sample".to_string())] + ); +} + +#[tokio::test] +async fn plugin_auth_projection_preserves_duplicate_connector_declaration_names() { + let codex_home = TempDir::new().unwrap(); + let plugin_root = codex_home + .path() + .join("plugins/cache") + .join("test") + .join("sample") + .join("local"); + write_file( + &plugin_root.join(".codex-plugin/plugin.json"), + r#"{"name":"sample"}"#, + ); + write_file( + &plugin_root.join(".mcp.json"), + r#"{ + "mcpServers": { + "foo": { + "type": "stdio", + "command": "foo-mcp" + }, + "foo2": { + "type": "stdio", + "command": "foo2-mcp" + }, + "other": { + "type": "stdio", + "command": "other-mcp" + } + } +}"#, + ); + write_file( + &plugin_root.join(".app.json"), + r#"{ + "apps": { + "foo": { + "id": "connector_shared" + }, + "foo2": { + "id": "connector_shared" + } + } +}"#, + ); + write_file( + &codex_home.path().join(CONFIG_TOML_FILE), + r#"[features] +plugins = true + +[plugins."sample@test"] +enabled = true +"#, + ); + let config = load_config(codex_home.path(), codex_home.path()).await; + let manager = PluginsManager::new_with_options( + codex_home.path().to_path_buf(), + Some(Product::Codex), + Some(AuthMode::Chatgpt), + ); + + let outcome = manager.plugins_for_config(&config).await; + + assert_eq!( + outcome.effective_apps(), + vec![AppConnectorId("connector_shared".to_string())] + ); + assert_eq!( + sorted_effective_mcp_server_names(&outcome), + vec!["other".to_string()] + ); + let sample = outcome + .capability_summaries() + .iter() + .find(|plugin| plugin.config_name == "sample@test") + .expect("sample plugin summary should exist"); + assert_eq!(sample.mcp_server_names, vec!["other".to_string()]); + assert_eq!( + sample.app_connector_ids, + vec![AppConnectorId("connector_shared".to_string())] + ); +} + #[tokio::test] async fn plugin_auth_projection_reprojects_cached_outcome_when_auth_changes() { let codex_home = TempDir::new().unwrap(); @@ -427,7 +563,7 @@ async fn load_plugins_loads_default_skills_and_mcp_servers() { let outcome = load_plugins_from_config( &plugin_config_toml(/*enabled*/ true, /*plugins_feature_enabled*/ true), codex_home.path(), - /*auth_mode*/ None, + Some(AuthMode::Chatgpt), ) .await; @@ -471,7 +607,7 @@ async fn load_plugins_loads_default_skills_and_mcp_servers() { tools: HashMap::new(), }, )]), - apps: Vec::new(), + apps: vec![app_declaration("example", "connector_example")], hook_sources: Vec::new(), hook_load_warnings: Vec::new(), error: None, @@ -485,7 +621,7 @@ async fn load_plugins_loads_default_skills_and_mcp_servers() { description: Some("Plugin that includes the sample MCP server and Skills".to_string(),), has_skills: true, mcp_server_names: vec!["sample".to_string()], - app_connector_ids: Vec::new(), + app_connector_ids: vec![AppConnectorId("connector_example".to_string())], }] ); assert_eq!( @@ -493,7 +629,10 @@ async fn load_plugins_loads_default_skills_and_mcp_servers() { vec![plugin_root.join("skills").abs()] ); assert_eq!(outcome.effective_mcp_servers().len(), 1); - assert!(outcome.effective_apps().is_empty()); + assert_eq!( + outcome.effective_apps(), + vec![AppConnectorId("connector_example".to_string())] + ); } #[tokio::test] @@ -1036,7 +1175,7 @@ async fn load_plugins_uses_manifest_configured_component_paths() { &plugin_root.join(".app.json"), r#"{ "apps": { - "default": { + "default-app": { "id": "connector_default" } } @@ -1046,7 +1185,7 @@ async fn load_plugins_uses_manifest_configured_component_paths() { &plugin_root.join("config/custom.app.json"), r#"{ "apps": { - "custom": { + "custom-app": { "id": "connector_custom" } } @@ -1056,7 +1195,7 @@ async fn load_plugins_uses_manifest_configured_component_paths() { let outcome = load_plugins_from_config( &plugin_config_toml(/*enabled*/ true, /*plugins_feature_enabled*/ true), codex_home.path(), - /*auth_mode*/ None, + Some(AuthMode::Chatgpt), ) .await; @@ -1095,7 +1234,10 @@ async fn load_plugins_uses_manifest_configured_component_paths() { }, )]) ); - assert!(outcome.plugins()[0].apps.is_empty()); + assert_eq!( + outcome.plugins()[0].apps, + vec![app_declaration("custom-app", "connector_custom")] + ); } #[tokio::test] @@ -1149,7 +1291,7 @@ async fn load_plugins_ignores_manifest_component_paths_without_dot_slash() { &plugin_root.join(".app.json"), r#"{ "apps": { - "default": { + "default-app": { "id": "connector_default" } } @@ -1159,7 +1301,7 @@ async fn load_plugins_ignores_manifest_component_paths_without_dot_slash() { &plugin_root.join("config/custom.app.json"), r#"{ "apps": { - "custom": { + "custom-app": { "id": "connector_custom" } } @@ -1169,7 +1311,7 @@ async fn load_plugins_ignores_manifest_component_paths_without_dot_slash() { let outcome = load_plugins_from_config( &plugin_config_toml(/*enabled*/ true, /*plugins_feature_enabled*/ true), codex_home.path(), - /*auth_mode*/ None, + Some(AuthMode::Chatgpt), ) .await; @@ -1205,7 +1347,10 @@ async fn load_plugins_ignores_manifest_component_paths_without_dot_slash() { }, )]) ); - assert!(outcome.plugins()[0].apps.is_empty()); + assert_eq!( + outcome.plugins()[0].apps, + vec![app_declaration("default-app", "connector_default")] + ); } #[tokio::test] @@ -1426,6 +1571,7 @@ async fn effective_apps_preserves_app_config_order() { fn capability_index_filters_inactive_and_zero_capability_plugins() { let codex_home = TempDir::new().unwrap(); let connector = |id: &str| AppConnectorId(id.to_string()); + let app = |name: &str, connector_id: &str| app_declaration(name, connector_id); let http_server = |url: &str| McpServerConfig { transport: McpServerTransportConfig::StreamableHttp { url: url.to_string(), @@ -1477,23 +1623,26 @@ fn capability_index_filters_inactive_and_zero_capability_plugins() { }, LoadedPlugin { mcp_servers: HashMap::from([("alpha".to_string(), http_server("https://alpha"))]), - apps: vec![connector("connector_example")], + apps: vec![app("example", "connector_example")], ..plugin("alpha@test", "alpha-plugin", "alpha-plugin") }, LoadedPlugin { mcp_servers: HashMap::from([("beta".to_string(), http_server("https://beta"))]), - apps: vec![connector("connector_example"), connector("connector_gmail")], + apps: vec![ + app("example", "connector_example"), + app("gmail", "connector_gmail"), + ], ..plugin("beta@test", "beta-plugin", "beta-plugin") }, plugin("empty@test", "empty-plugin", "empty-plugin"), LoadedPlugin { enabled: false, skill_roots: vec![codex_home.path().join("disabled-plugin/skills").abs()], - apps: vec![connector("connector_hidden")], + apps: vec![app("hidden", "connector_hidden")], ..plugin("disabled@test", "disabled-plugin", "disabled-plugin") }, LoadedPlugin { - apps: vec![connector("connector_broken")], + apps: vec![app("broken", "connector_broken")], error: Some("failed to load".to_string()), ..plugin("broken@test", "broken-plugin", "broken-plugin") }, @@ -2461,7 +2610,18 @@ async fn read_plugin_for_config_installed_git_source_reads_from_cache_without_cl ); write_file( &cached_plugin_root.join(".app.json"), - r#"{"apps":{"calendar":{"id":"connector_calendar"}}}"#, + r#"{ + "apps": { + "calendar": { + "id": "connector_calendar", + "category": "First Category" + }, + "calendar_duplicate": { + "id": "connector_calendar", + "category": "Second Category" + } + } +}"#, ); write_file( &cached_plugin_root.join(".mcp.json"), @@ -2546,6 +2706,13 @@ enabled = false outcome.plugin.apps, vec![AppConnectorId("connector_calendar".to_string())] ); + assert_eq!( + outcome.plugin.app_category_by_id, + HashMap::from([( + "connector_calendar".to_string(), + "First Category".to_string() + )]) + ); assert_eq!( outcome.plugin.hooks, vec![ diff --git a/codex-rs/core/tests/suite/plugins.rs b/codex-rs/core/tests/suite/plugins.rs index eafc1c646..f13c2302f 100644 --- a/codex-rs/core/tests/suite/plugins.rs +++ b/codex-rs/core/tests/suite/plugins.rs @@ -86,16 +86,22 @@ fn write_plugin_mcp_plugin(home: &TempDir, command: &str) { } fn write_plugin_app_plugin(home: &TempDir) { + write_plugin_app_plugin_with_name(home, "sample"); +} + +fn write_plugin_app_plugin_with_name(home: &TempDir, app_name: &str) { let plugin_root = write_sample_plugin_manifest_and_config(home); std::fs::write( plugin_root.join(".app.json"), - r#"{ - "apps": { - "calendar": { + format!( + r#"{{ + "apps": {{ + "{app_name}": {{ "id": "calendar" - } - } -}"#, + }} + }} +}}"# + ), ) .expect("write plugin app config"); } @@ -316,6 +322,86 @@ async fn explicit_plugin_mentions_use_apps_for_chatgpt_dual_surface_plugins() -> Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn explicit_plugin_mentions_keep_non_conflicting_mcp_for_chatgpt_auth() -> Result<()> { + skip_if_no_network!(Ok(())); + let server = start_mock_server().await; + let apps_server = AppsTestServer::mount_with_connector_name(&server, "Google Calendar").await?; + let mock = mount_sse_once( + &server, + sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]), + ) + .await; + + let codex_home = Arc::new(TempDir::new()?); + let rmcp_test_server_bin = match stdio_server_bin() { + Ok(bin) => bin, + Err(err) => { + eprintln!("test_stdio_server binary not available, skipping test: {err}"); + return Ok(()); + } + }; + write_plugin_skill_plugin(codex_home.as_ref()); + write_plugin_mcp_plugin(codex_home.as_ref(), &rmcp_test_server_bin); + write_plugin_app_plugin_with_name(codex_home.as_ref(), "sample_app"); + + let test_codex = + build_apps_enabled_plugin_test_codex(&server, codex_home, apps_server.chatgpt_base_url) + .await?; + let codex = Arc::clone(&test_codex.codex); + wait_for_mcp_server(&codex, "sample").await?; + + codex + .submit(Op::UserInput { + items: vec![codex_protocol::user_input::UserInput::Mention { + name: "sample".into(), + path: format!("plugin://{SAMPLE_PLUGIN_CONFIG_NAME}"), + }], + final_output_json_schema: None, + responsesapi_client_metadata: None, + additional_context: Default::default(), + thread_settings: Default::default(), + }) + .await?; + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; + + let request = mock.single_request(); + let developer_messages = request.message_input_texts("developer"); + assert!( + developer_messages + .iter() + .any(|text| text.contains("MCP servers from this plugin")), + "expected plugin MCP guidance to remain visible for non-conflicting app declaration: {developer_messages:?}" + ); + assert!( + developer_messages + .iter() + .any(|text| text.contains("Apps from this plugin")), + "expected plugin app guidance: {developer_messages:?}" + ); + let request_body = request.body_json(); + let request_tools = tool_names(&request_body); + assert!( + request_tools + .iter() + .any(|name| name == "mcp__codex_apps__google_calendar"), + "expected plugin app tools to become visible for this turn: {request_tools:?}" + ); + let echo_tool = request + .tool_by_name("mcp__sample", "echo") + .expect("plugin MCP tool should remain present"); + let echo_description = echo_tool + .get("description") + .and_then(serde_json::Value::as_str) + .expect("plugin MCP tool description should be present"); + assert!( + echo_description.contains("This tool is part of plugin `sample`."), + "expected plugin MCP provenance in tool description: {echo_description:?}" + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn explicit_plugin_mentions_use_mcp_for_api_key_dual_surface_plugins() -> Result<()> { skip_if_no_network!(Ok(())); diff --git a/codex-rs/plugin/src/lib.rs b/codex-rs/plugin/src/lib.rs index 7bd4ac648..7daa78bc0 100644 --- a/codex-rs/plugin/src/lib.rs +++ b/codex-rs/plugin/src/lib.rs @@ -1,5 +1,7 @@ //! Shared plugin package models, source providers, identifiers, and telemetry summaries. +use std::collections::HashSet; + pub use codex_utils_plugins::mention_syntax; pub use codex_utils_plugins::plugin_namespace_for_skill_path; @@ -26,6 +28,26 @@ pub use provider::ResolvedPluginLocation; #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct AppConnectorId(pub String); +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct AppDeclaration { + pub name: String, + pub connector_id: AppConnectorId, + pub category: Option, +} + +pub fn app_connector_ids_from_declarations<'a>( + app_declarations: impl IntoIterator, +) -> Vec { + let mut connector_ids = Vec::new(); + let mut seen_connector_ids = HashSet::new(); + for app in app_declarations { + if seen_connector_ids.insert(&app.connector_id) { + connector_ids.push(app.connector_id.clone()); + } + } + connector_ids +} + #[derive(Debug, Clone, Default, PartialEq, Eq)] pub struct PluginCapabilitySummary { pub config_name: String, diff --git a/codex-rs/plugin/src/load_outcome.rs b/codex-rs/plugin/src/load_outcome.rs index 2588ee0a7..5d731a8b5 100644 --- a/codex-rs/plugin/src/load_outcome.rs +++ b/codex-rs/plugin/src/load_outcome.rs @@ -5,8 +5,10 @@ use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_plugins::PluginSkillRoot; use crate::AppConnectorId; +use crate::AppDeclaration; use crate::PluginCapabilitySummary; use crate::PluginHookSource; +use crate::app_connector_ids_from_declarations; const MAX_CAPABILITY_SUMMARY_DESCRIPTION_LEN: usize = 1024; @@ -22,7 +24,7 @@ pub struct LoadedPlugin { pub disabled_skill_paths: HashSet, pub has_enabled_skills: bool, pub mcp_servers: HashMap, - pub apps: Vec, + pub apps: Vec, pub hook_sources: Vec, pub hook_load_warnings: Vec, pub error: Option, @@ -53,7 +55,7 @@ fn plugin_capability_summary_from_loaded( description: prompt_safe_plugin_description(plugin.manifest_description.as_deref()), has_skills: plugin.has_enabled_skills, mcp_server_names, - app_connector_ids: plugin.apps.clone(), + app_connector_ids: app_connector_ids_from_declarations(&plugin.apps), }; (summary.has_skills @@ -149,18 +151,12 @@ impl PluginLoadOutcome { } pub fn effective_apps(&self) -> Vec { - let mut apps = Vec::new(); - let mut seen_connector_ids = HashSet::new(); - - for plugin in self.plugins.iter().filter(|plugin| plugin.is_active()) { - for connector_id in &plugin.apps { - if seen_connector_ids.insert(connector_id.clone()) { - apps.push(connector_id.clone()); - } - } - } - - apps + app_connector_ids_from_declarations( + self.plugins + .iter() + .filter(|plugin| plugin.is_active()) + .flat_map(|plugin| plugin.apps.iter()), + ) } pub fn effective_plugin_hook_sources(&self) -> Vec {