diff --git a/codex-rs/app-server/tests/suite/v2/recommended_plugins.rs b/codex-rs/app-server/tests/suite/v2/recommended_plugins.rs index d632af2140..a237875d5b 100644 --- a/codex-rs/app-server/tests/suite/v2/recommended_plugins.rs +++ b/codex-rs/app-server/tests/suite/v2/recommended_plugins.rs @@ -55,7 +55,7 @@ async fn recommended_plugins_after_external_login( let server = responses::start_mock_server().await; let apps_server = AppsTestServer::mount(&server).await?; Mock::given(method("GET")) - .and(path("/ps/plugins/suggested")) + .and(path("/ps/plugins/suggested/codex")) .and(query_param("scope", "GLOBAL")) .respond_with( ResponseTemplate::new(200) @@ -65,9 +65,7 @@ async fn recommended_plugins_after_external_login( "plugins": [{ "id": "plugin_github", "name": "github", - "status": "ENABLED", - "installation_policy": "AVAILABLE", - "release": {"display_name": "GitHub"} + "display_name": "GitHub" }] })), ) diff --git a/codex-rs/core-plugins/src/lib.rs b/codex-rs/core-plugins/src/lib.rs index cedca7c075..5c84f19801 100644 --- a/codex-rs/core-plugins/src/lib.rs +++ b/codex-rs/core-plugins/src/lib.rs @@ -18,6 +18,7 @@ mod plugin_bundle_archive; mod plugin_metrics; mod plugin_metrics_sidecar; mod provider; +mod recommended_plugin_install; pub mod remote; pub mod remote_bundle; pub mod remote_legacy; @@ -86,6 +87,7 @@ pub use plugin_metrics_sidecar::strip_output_env; pub use provider::ExecutorPluginProvider; pub use provider::ExecutorPluginProviderError; pub use provider::ResolvedExecutorPlugin; +pub use recommended_plugin_install::hydrate_selected_recommended_plugin_install_metadata; pub use remote::RecommendedPlugin; pub use remote::RecommendedPluginsMode; pub use script_attribution::PluginCommandAttribution; diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index a0dc23ad08..c23a3b6b53 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -1373,7 +1373,7 @@ impl PluginsManager { description: None, has_skills: false, mcp_server_names: Vec::new(), - app_connector_ids: plugin.app_connector_ids, + app_connector_ids: Vec::new(), }) }) .collect(); diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index 277722eebf..b5088686ab 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -5618,7 +5618,7 @@ plugins = true let server = MockServer::start().await; Mock::given(method("GET")) - .and(path("/ps/plugins/suggested")) + .and(path("/ps/plugins/suggested/codex")) .and(query_param("scope", "GLOBAL")) .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ "enabled": true, @@ -5678,7 +5678,7 @@ plugins = true let server = MockServer::start().await; Mock::given(method("GET")) - .and(path("/ps/plugins/suggested")) + .and(path("/ps/plugins/suggested/codex")) .and(query_param("scope", "GLOBAL")) .and(header("authorization", "Bearer Access Token")) .and(header("chatgpt-account-id", "account_id")) @@ -5691,15 +5691,12 @@ plugins = true { "id": "plugin_slack", "name": "slack", - "release": { - "display_name": "Slack", - "app_ids": ["connector_slack"] - } + "display_name": "Slack" }, { "id": "plugin_github", "name": "github", - "release": {"display_name": "GitHub"} + "display_name": "GitHub" } ] })) @@ -5719,13 +5716,11 @@ plugins = true config_id: "github@openai-curated-remote".to_string(), remote_plugin_id: "plugin_github".to_string(), display_name: "GitHub".to_string(), - app_connector_ids: Vec::new(), }, RecommendedPlugin { config_id: "slack@openai-curated-remote".to_string(), remote_plugin_id: "plugin_slack".to_string(), display_name: "Slack".to_string(), - app_connector_ids: vec!["connector_slack".to_string()], }, ], }; @@ -5754,24 +5749,24 @@ plugins = true ); let server = MockServer::start().await; Mock::given(method("GET")) - .and(path("/ps/plugins/suggested")) + .and(path("/ps/plugins/suggested/codex")) .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ "enabled": true, "plugins": [ { "id": "plugin_linear", "name": "linear", - "release": {"display_name": "Linear"} + "display_name": "Linear" }, { "id": "plugin_github", "name": "github", - "release": {"display_name": "GitHub"} + "display_name": "GitHub" }, { "id": "plugin_slack", "name": "slack", - "release": {"display_name": "Slack"} + "display_name": "Slack" } ] }))) @@ -5827,7 +5822,7 @@ plugins = true let server = MockServer::start().await; Mock::given(method("GET")) - .and(path("/ps/plugins/suggested")) + .and(path("/ps/plugins/suggested/codex")) .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ "enabled": false, "plugins": [] @@ -5866,7 +5861,7 @@ plugins = true let server = MockServer::start().await; Mock::given(method("GET")) - .and(path("/ps/plugins/suggested")) + .and(path("/ps/plugins/suggested/codex")) .respond_with(ResponseTemplate::new(500).set_body_string("unavailable")) .expect(1) .mount(&server) @@ -5885,7 +5880,7 @@ plugins = true server.reset().await; Mock::given(method("GET")) - .and(path("/ps/plugins/suggested")) + .and(path("/ps/plugins/suggested/codex")) .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ "enabled": true, "plugins": [] diff --git a/codex-rs/core-plugins/src/recommended_plugin_install.rs b/codex-rs/core-plugins/src/recommended_plugin_install.rs new file mode 100644 index 0000000000..b63846d3f3 --- /dev/null +++ b/codex-rs/core-plugins/src/recommended_plugin_install.rs @@ -0,0 +1,31 @@ +use crate::PluginsConfigInput; +use crate::remote::RemotePluginCatalogError; +use codex_login::CodexAuth; +use codex_tools::DiscoverableTool; + +pub async fn hydrate_selected_recommended_plugin_install_metadata( + config: &PluginsConfigInput, + auth: Option<&CodexAuth>, + mut tool: DiscoverableTool, +) -> Result, RemotePluginCatalogError> { + let DiscoverableTool::Plugin(plugin) = &mut tool else { + return Ok(Some(tool)); + }; + let remote_plugin_id = plugin.remote_plugin_id.clone().ok_or_else(|| { + RemotePluginCatalogError::UnexpectedResponse(format!( + "recommended plugin `{}` is missing remote plugin identity", + plugin.id + )) + })?; + let Some(app_connector_ids) = crate::remote::fetch_recommended_plugin_install_metadata( + &config.remote_plugin_service_config(), + auth, + &remote_plugin_id, + ) + .await? + else { + return Ok(None); + }; + plugin.app_connector_ids = app_connector_ids; + Ok(Some(tool)) +} diff --git a/codex-rs/core-plugins/src/remote.rs b/codex-rs/core-plugins/src/remote.rs index b7d4c176d0..0e4434ee9f 100644 --- a/codex-rs/core-plugins/src/remote.rs +++ b/codex-rs/core-plugins/src/remote.rs @@ -336,7 +336,6 @@ pub struct RecommendedPlugin { pub config_id: String, pub remote_plugin_id: String, pub display_name: String, - pub app_connector_ids: Vec, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -749,9 +748,7 @@ struct RemotePluginListResponse { #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] struct RecommendedPluginsResponse { - #[serde(default)] - enabled: Option, - #[serde(default)] + enabled: bool, plugins: Vec, } @@ -759,18 +756,7 @@ struct RecommendedPluginsResponse { struct RecommendedPluginItem { id: String, name: String, - #[serde(default)] - status: Option, - #[serde(default)] - installation_policy: Option, - release: RecommendedPluginRelease, -} - -#[derive(Debug, Clone, PartialEq, Eq, Deserialize)] -struct RecommendedPluginRelease { display_name: String, - #[serde(default)] - app_ids: Vec, } #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] @@ -996,7 +982,7 @@ pub async fn fetch_recommended_plugins( ) -> Result { let auth = ensure_chatgpt_auth(auth)?; let base_url = config.chatgpt_base_url.trim_end_matches('/'); - let mut url = Url::parse(&format!("{base_url}/ps/plugins/suggested")) + let mut url = Url::parse(&format!("{base_url}/ps/plugins/suggested/codex")) .map_err(RemotePluginCatalogError::InvalidBaseUrl)?; url.query_pairs_mut().append_pair("scope", "GLOBAL"); let url = url.to_string(); @@ -1006,8 +992,48 @@ pub async fn fetch_recommended_plugins( Ok(recommended_plugins_mode(response)) } +#[instrument(level = "trace", skip_all)] +pub(crate) async fn fetch_recommended_plugin_install_metadata( + config: &RemotePluginServiceConfig, + auth: Option<&CodexAuth>, + plugin_id: &str, +) -> Result>, RemotePluginCatalogError> { + let auth = ensure_chatgpt_auth(auth)?; + let plugin = match fetch_plugin_detail_with_timeout( + config, + auth, + plugin_id, + /*include_download_urls*/ false, + RECOMMENDED_PLUGINS_TIMEOUT, + ) + .await + { + Ok(plugin) => plugin, + Err(RemotePluginCatalogError::UnexpectedStatus { + status: StatusCode::NOT_FOUND, + .. + }) => return Ok(None), + Err(err) => return Err(err), + }; + if plugin.id != plugin_id { + return Err(RemotePluginCatalogError::UnexpectedPluginId { + expected: plugin_id.to_string(), + actual: plugin.id, + }); + } + if plugin.availability != PluginAvailability::Available + || plugin.installation_policy != PluginInstallPolicy::Available + { + return Ok(None); + } + + let (app_connector_ids, _) = + effective_remote_plugin_apps_and_mcp_servers(&plugin.release, auth); + Ok(Some(app_connector_ids)) +} + fn recommended_plugins_mode(response: RecommendedPluginsResponse) -> RecommendedPluginsMode { - if response.enabled != Some(true) { + if !response.enabled { return RecommendedPluginsMode::Legacy; } @@ -1015,12 +1041,6 @@ fn recommended_plugins_mode(response: RecommendedPluginsResponse) -> Recommended for plugin in response.plugins { if !is_valid_remote_plugin_id(&plugin.id) || plugin.name.chars().count() > MAX_RECOMMENDED_PLUGIN_NAME_LEN - || plugin - .status - .is_some_and(|status| status != PluginAvailability::Available) - || plugin - .installation_policy - .is_some_and(|policy| policy != PluginInstallPolicy::Available) { continue; } @@ -1038,20 +1058,11 @@ fn recommended_plugins_mode(response: RecommendedPluginsResponse) -> Recommended continue; } }; - let RecommendedPluginRelease { - display_name, - app_ids, - } = plugin.release; - let display_name = non_empty_string(Some(&display_name)) + let display_name = non_empty_string(Some(&plugin.display_name)) .unwrap_or_else(|| plugin.name.clone()) .chars() .take(MAX_RECOMMENDED_PLUGIN_DISPLAY_NAME_LEN) .collect(); - let mut seen_app_ids = HashSet::new(); - let app_connector_ids = app_ids - .into_iter() - .filter(|app_id| !app_id.is_empty() && seen_app_ids.insert(app_id.clone())) - .collect(); let config_id = plugin_id.as_key(); plugins .entry(config_id.clone()) @@ -1059,7 +1070,6 @@ fn recommended_plugins_mode(response: RecommendedPluginsResponse) -> Recommended config_id, remote_plugin_id: plugin.id, display_name, - app_connector_ids, }); } @@ -1406,31 +1416,8 @@ async fn build_remote_plugin_detail( enabled: !disabled_skill_names.contains(&skill.name), }) .collect(); - let mut app_declarations = plugin - .release - .app_manifest - .as_ref() - .map(plugin_app_declarations_from_value) - .unwrap_or_else(|| app_declarations_from_remote_app_ids(&plugin.release.app_ids)); - let mut mcp_servers = plugin - .release - .mcp_servers - .iter() - .map(|server| (server.key.clone(), ())) - .collect::>(); - apply_app_mcp_routing_policy( - &mut app_declarations, - &mut mcp_servers, - Some(auth.api_auth_mode()), - /*plugin_active*/ true, - ); - let app_ids = app_connector_ids_from_declarations(&app_declarations) - .into_iter() - .map(|app_id| app_id.0) - .collect(); - let mut mcp_servers = mcp_servers.into_keys().collect::>(); - mcp_servers.sort_unstable(); - mcp_servers.dedup(); + let (app_ids, mcp_servers) = + effective_remote_plugin_apps_and_mcp_servers(&plugin.release, auth); Ok(RemotePluginDetail { marketplace_name, @@ -1475,6 +1462,36 @@ fn app_declarations_from_remote_app_ids(app_ids: &[String]) -> Vec (Vec, Vec) { + let mut app_declarations = release + .app_manifest + .as_ref() + .map(plugin_app_declarations_from_value) + .unwrap_or_else(|| app_declarations_from_remote_app_ids(&release.app_ids)); + let mut mcp_servers = release + .mcp_servers + .iter() + .map(|server| (server.key.clone(), ())) + .collect::>(); + apply_app_mcp_routing_policy( + &mut app_declarations, + &mut mcp_servers, + Some(auth.api_auth_mode()), + /*plugin_active*/ true, + ); + let app_ids = app_connector_ids_from_declarations(&app_declarations) + .into_iter() + .map(|app_id| app_id.0) + .collect(); + let mut mcp_server_names = mcp_servers.into_keys().collect::>(); + mcp_server_names.sort_unstable(); + mcp_server_names.dedup(); + (app_ids, mcp_server_names) +} + #[derive(Serialize)] struct RemotePluginInstallRequest<'a> { install_attempt_id: &'a str, @@ -2172,6 +2189,23 @@ async fn fetch_plugin_detail( auth: &CodexAuth, plugin_id: &str, include_download_urls: bool, +) -> Result { + fetch_plugin_detail_with_timeout( + config, + auth, + plugin_id, + include_download_urls, + REMOTE_PLUGIN_CATALOG_TIMEOUT, + ) + .await +} + +async fn fetch_plugin_detail_with_timeout( + config: &RemotePluginServiceConfig, + auth: &CodexAuth, + plugin_id: &str, + include_download_urls: bool, + timeout: Duration, ) -> Result { let base_url = config.chatgpt_base_url.trim_end_matches('/'); let mut url = Url::parse(&format!("{base_url}/ps/plugins/{plugin_id}")) @@ -2181,7 +2215,8 @@ async fn fetch_plugin_detail( .append_pair("includeDownloadUrls", "true"); } let url = url.to_string(); - let request = authenticated_request(config.http_request(Method::GET, &url), auth); + let request = + authenticated_request(config.http_request(Method::GET, &url), auth).timeout(timeout); send_and_decode(request, &url).await } diff --git a/codex-rs/core-plugins/src/remote_tests.rs b/codex-rs/core-plugins/src/remote_tests.rs index eac1c6e03d..6828084b24 100644 --- a/codex-rs/core-plugins/src/remote_tests.rs +++ b/codex-rs/core-plugins/src/remote_tests.rs @@ -48,6 +48,42 @@ async fn remote_plugin_list_routes_the_complete_query_url() { ); } +#[tokio::test] +async fn recommended_plugins_requests_codex_suggestions_endpoint() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/backend-api/ps/plugins/suggested/codex")) + .and(query_param("scope", "GLOBAL")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "enabled": true, + "plugins": [], + }))) + .expect(1) + .mount(&server) + .await; + let (config, selected_urls) = + recording_remote_plugin_service_config(format!("{}/backend-api", server.uri())); + let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); + + let mode = fetch_recommended_plugins(&config, Some(&auth)) + .await + .expect("recommended plugin request should succeed"); + + assert_eq!( + mode, + RecommendedPluginsMode::Endpoint { + plugins: Vec::new(), + } + ); + assert_eq!( + recorded_http_client_urls(&selected_urls), + vec![format!( + "{}/backend-api/ps/plugins/suggested/codex?scope=GLOBAL", + server.uri() + )] + ); +} + #[tokio::test] async fn remote_installed_plugins_paginate_across_all_scopes_without_download_urls() { let server = MockServer::start().await; @@ -483,16 +519,11 @@ fn remote_plugin_interface_maps_dark_logo_url() { Some("https://example.com/linear/logo-dark.png".to_string()) ); } -fn item(name: &str, display_name: &str) -> RecommendedPluginItem { +fn item(name: &str) -> RecommendedPluginItem { RecommendedPluginItem { id: format!("plugin_{name}"), name: name.to_string(), - status: None, - installation_policy: None, - release: RecommendedPluginRelease { - display_name: display_name.to_string(), - app_ids: Vec::new(), - }, + display_name: name.to_string(), } } @@ -500,7 +531,7 @@ fn item(name: &str, display_name: &str) -> RecommendedPluginItem { fn recommended_plugins_enabled_flag_selects_endpoint_or_legacy_mode() { let disabled: RecommendedPluginsResponse = serde_json::from_value(serde_json::json!({ "enabled": false, - "plugins": [{"id": "plugin_github", "name": "github", "release": {"display_name": "GitHub"}}] + "plugins": [{"id": "plugin_github", "name": "github", "display_name": "GitHub"}] })) .expect("response should deserialize"); assert_eq!( @@ -508,18 +539,6 @@ fn recommended_plugins_enabled_flag_selects_endpoint_or_legacy_mode() { RecommendedPluginsMode::Legacy ); - for response in [ - serde_json::json!({"plugins": []}), - serde_json::json!({"enabled": null, "plugins": []}), - ] { - let response: RecommendedPluginsResponse = - serde_json::from_value(response).expect("response should deserialize"); - assert_eq!( - recommended_plugins_mode(response), - RecommendedPluginsMode::Legacy - ); - } - let enabled: RecommendedPluginsResponse = serde_json::from_value(serde_json::json!({ "enabled": true, "plugins": [] @@ -537,10 +556,7 @@ fn recommended_plugins_enabled_flag_selects_endpoint_or_legacy_mode() { fn recommended_plugins_require_remote_install_identity() { let response = serde_json::from_value::(serde_json::json!({ "enabled": true, - "plugins": [{ - "name": "github", - "release": {"display_name": "GitHub"} - }] + "plugins": [{"name": "github", "display_name": "GitHub"}] })); assert!(response.is_err()); @@ -550,33 +566,13 @@ fn recommended_plugins_require_remote_install_identity() { fn recommended_plugins_are_validated_deduplicated_sorted_and_capped() { let mut plugins = (0..=52) .rev() - .map(|index| item(&format!("plugin-{index:02}"), &format!("Plugin {index:02}"))) + .map(|index| item(&format!("plugin-{index:02}"))) .collect::>(); - plugins.push(item("plugin-00", "Duplicate")); - plugins.push(item("not/a/plugin", "Invalid")); - plugins.push(RecommendedPluginItem { - id: "plugin_disabled".to_string(), - name: "disabled".to_string(), - status: Some(PluginAvailability::DisabledByAdmin), - installation_policy: Some(PluginInstallPolicy::Available), - release: RecommendedPluginRelease { - display_name: "Disabled".to_string(), - app_ids: Vec::new(), - }, - }); - plugins.push(RecommendedPluginItem { - id: "plugin_not_available".to_string(), - name: "not-available".to_string(), - status: Some(PluginAvailability::Available), - installation_policy: Some(PluginInstallPolicy::NotAvailable), - release: RecommendedPluginRelease { - display_name: "Not Available".to_string(), - app_ids: Vec::new(), - }, - }); + plugins.push(item("plugin-00")); + plugins.push(item("not/a/plugin")); let mode = recommended_plugins_mode(RecommendedPluginsResponse { - enabled: Some(true), + enabled: true, plugins, }); let RecommendedPluginsMode::Endpoint { plugins } = mode else { @@ -589,8 +585,7 @@ fn recommended_plugins_are_validated_deduplicated_sorted_and_capped() { Some(&RecommendedPlugin { config_id: "plugin-00@openai-curated-remote".to_string(), remote_plugin_id: "plugin_plugin-00".to_string(), - display_name: "Plugin 00".to_string(), - app_connector_ids: Vec::new(), + display_name: "plugin-00".to_string(), }) ); assert_eq!( @@ -598,8 +593,7 @@ fn recommended_plugins_are_validated_deduplicated_sorted_and_capped() { Some(&RecommendedPlugin { config_id: "plugin-49@openai-curated-remote".to_string(), remote_plugin_id: "plugin_plugin-49".to_string(), - display_name: "Plugin 49".to_string(), - app_connector_ids: Vec::new(), + display_name: "plugin-49".to_string(), }) ); } @@ -609,55 +603,56 @@ fn recommended_plugins_bound_model_visible_fields() { let overlong_name = "n".repeat(MAX_RECOMMENDED_PLUGIN_NAME_LEN + 1); let overlong_display_name = "D".repeat(MAX_RECOMMENDED_PLUGIN_DISPLAY_NAME_LEN + 1); let mode = recommended_plugins_mode(RecommendedPluginsResponse { - enabled: Some(true), + enabled: true, plugins: vec![ - item(&overlong_name, "Ignored"), - item("bounded", &overlong_display_name), + item(&overlong_name), + RecommendedPluginItem { + id: "plugin_bounded".to_string(), + name: "bounded".to_string(), + display_name: overlong_display_name, + }, + RecommendedPluginItem { + id: "plugin_fallback".to_string(), + name: "fallback".to_string(), + display_name: String::new(), + }, ], }); assert_eq!( mode, RecommendedPluginsMode::Endpoint { - plugins: vec![RecommendedPlugin { - config_id: "bounded@openai-curated-remote".to_string(), - remote_plugin_id: "plugin_bounded".to_string(), - display_name: "D".repeat(MAX_RECOMMENDED_PLUGIN_DISPLAY_NAME_LEN), - app_connector_ids: Vec::new(), - }], + plugins: vec![ + RecommendedPlugin { + config_id: "bounded@openai-curated-remote".to_string(), + remote_plugin_id: "plugin_bounded".to_string(), + display_name: "D".repeat(MAX_RECOMMENDED_PLUGIN_DISPLAY_NAME_LEN), + }, + RecommendedPlugin { + config_id: "fallback@openai-curated-remote".to_string(), + remote_plugin_id: "plugin_fallback".to_string(), + display_name: "fallback".to_string(), + }, + ], } ); } #[test] -fn recommended_plugins_preserve_install_identity_and_normalize_app_ids() { - let mode = recommended_plugins_mode(RecommendedPluginsResponse { - enabled: Some(true), - plugins: vec![RecommendedPluginItem { - id: "plugin_connector_sample".to_string(), - name: "sample".to_string(), - status: Some(PluginAvailability::Available), - installation_policy: Some(PluginInstallPolicy::Available), - release: RecommendedPluginRelease { - display_name: "Sample".to_string(), - app_ids: vec![ - "connector_one".to_string(), - String::new(), - "connector_two".to_string(), - "connector_one".to_string(), - ], - }, - }], - }); +fn recommended_plugins_accept_codex_suggestion_items() { + let response: RecommendedPluginsResponse = serde_json::from_value(serde_json::json!({ + "enabled": true, + "plugins": [{"id": "plugin_box", "name": "box", "display_name": "Box"}] + })) + .expect("compact response should deserialize"); assert_eq!( - mode, + recommended_plugins_mode(response), RecommendedPluginsMode::Endpoint { plugins: vec![RecommendedPlugin { - config_id: "sample@openai-curated-remote".to_string(), - remote_plugin_id: "plugin_connector_sample".to_string(), - display_name: "Sample".to_string(), - app_connector_ids: vec!["connector_one".to_string(), "connector_two".to_string(),], + config_id: "box@openai-curated-remote".to_string(), + remote_plugin_id: "plugin_box".to_string(), + display_name: "Box".to_string(), }], } ); @@ -666,16 +661,11 @@ fn recommended_plugins_preserve_install_identity_and_normalize_app_ids() { #[test] fn recommended_plugins_ignore_invalid_remote_plugin_ids() { let mode = recommended_plugins_mode(RecommendedPluginsResponse { - enabled: Some(true), + enabled: true, plugins: vec![RecommendedPluginItem { id: "not/a/plugin".to_string(), name: "sample".to_string(), - status: None, - installation_policy: None, - release: RecommendedPluginRelease { - display_name: "Sample".to_string(), - app_ids: Vec::new(), - }, + display_name: "Sample".to_string(), }], }); diff --git a/codex-rs/core/src/tools/handlers/request_plugin_install.rs b/codex-rs/core/src/tools/handlers/request_plugin_install.rs index 50739c669d..6236423d23 100644 --- a/codex-rs/core/src/tools/handlers/request_plugin_install.rs +++ b/codex-rs/core/src/tools/handlers/request_plugin_install.rs @@ -177,6 +177,31 @@ impl RequestPluginInstallHandler { "{argument_name} must match one of {source}" )) })?; + let tool = if self.presentation == ToolSuggestPresentation::RecommendationContext { + let plugin_id = tool.id().to_string(); + let auth = session.services.auth_manager.auth().await; + let plugins_config = turn.config.plugins_config_input(); + match codex_core_plugins::hydrate_selected_recommended_plugin_install_metadata( + &plugins_config, + auth.as_ref(), + tool, + ) + .await + { + Ok(Some(tool)) => tool, + Ok(None) => return Err(recommended_plugins_no_longer_available()), + Err(err) => { + warn!( + plugin_id, + error = %err, + "failed to hydrate selected recommended plugin install metadata" + ); + return Err(recommended_plugin_metadata_retryable()); + } + } + } else { + tool + }; let tool_type = tool.tool_type(); let suggestion_id = format!("request_plugin_install_{call_id}"); @@ -286,6 +311,18 @@ impl RequestPluginInstallHandler { } } +fn recommended_plugins_no_longer_available() -> FunctionCallError { + FunctionCallError::RespondToModel( + "The recommended plugins for this turn are no longer available.".to_string(), + ) +} + +fn recommended_plugin_metadata_retryable() -> FunctionCallError { + FunctionCallError::RespondToModel( + "The selected recommended plugin could not be verified right now. Retry request_plugin_install with the same plugin_id.".to_string(), + ) +} + impl CoreToolRuntime for RequestPluginInstallHandler {} async fn maybe_persist_disabled_install_request( diff --git a/codex-rs/core/tests/suite/request_plugin_install.rs b/codex-rs/core/tests/suite/request_plugin_install.rs index cdf58a608d..b70e5b5ca9 100644 --- a/codex-rs/core/tests/suite/request_plugin_install.rs +++ b/codex-rs/core/tests/suite/request_plugin_install.rs @@ -119,7 +119,7 @@ fn configure_apps_without_search_tool(config: &mut Config, apps_base_url: &str) async fn mount_recommendations(server: &wiremock::MockServer, response: ResponseTemplate) { Mock::given(method("GET")) - .and(path("/ps/plugins/suggested")) + .and(path("/ps/plugins/suggested/codex")) .and(query_param("scope", "GLOBAL")) .respond_with(response) .mount(server) @@ -209,7 +209,7 @@ async fn start_gated_step_preparation(test: &TestCodex, server: &MockServer) -> .await .unwrap_or_default() .into_iter() - .filter(|request| request.url.path() == "/ps/plugins/suggested") + .filter(|request| request.url.path() == "/ps/plugins/suggested/codex") .count(); let (sandbox_policy, permission_profile) = turn_permission_fields(PermissionProfile::Disabled, test.config.cwd.as_path()); @@ -241,7 +241,7 @@ async fn start_gated_step_preparation(test: &TestCodex, server: &MockServer) -> .await .unwrap_or_default() .into_iter() - .filter(|request| request.url.path() == "/ps/plugins/suggested") + .filter(|request| request.url.path() == "/ps/plugins/suggested/codex") .count(); if mcp_started && recommendation_count > prior_recommendation_count { break; @@ -312,24 +312,38 @@ async fn resolve_install_elicitation( async fn mount_remote_calendar_recommendation(server: &wiremock::MockServer) { Mock::given(method("GET")) - .and(path("/ps/plugins/suggested")) + .and(path("/ps/plugins/suggested/codex")) .and(query_param("scope", "GLOBAL")) .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "enabled": true, "plugins": [{ "id": REMOTE_CALENDAR_PLUGIN_ID, "name": "calendar", - "status": "ENABLED", - "installation_policy": "AVAILABLE", - "release": { - "display_name": "Calendar", - "app_ids": [CALENDAR_CONNECTOR_ID] - } + "display_name": "Calendar" }] }))) .expect(1) .mount(server) .await; + Mock::given(method("GET")) + .and(path(format!("/ps/plugins/{REMOTE_CALENDAR_PLUGIN_ID}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "id": REMOTE_CALENDAR_PLUGIN_ID, + "name": "calendar", + "scope": "GLOBAL", + "status": "ENABLED", + "installation_policy": "AVAILABLE", + "authentication_policy": "ON_USE", + "release": { + "display_name": "Calendar", + "description": "Manage calendar events.", + "app_ids": [CALENDAR_CONNECTOR_ID], + "interface": {} + } + }))) + .expect(1) + .mount(server) + .await; } fn remote_installed_plugins_response(plugins: Vec) -> ResponseTemplate { @@ -388,9 +402,7 @@ async fn mcp_discovery_overlaps_endpoint_plugin_recommendations() -> Result<()> "plugins": [{ "id": "plugin_github", "name": "github", - "status": "ENABLED", - "installation_policy": "AVAILABLE", - "release": {"display_name": "GitHub"} + "display_name": "GitHub" }] })), ) @@ -671,16 +683,12 @@ async fn endpoint_mode_injects_candidates_hides_list_and_rejects_invented_ids() { "id": "plugin_google_calendar", "name": "google-calendar", - "status": "ENABLED", - "installation_policy": "AVAILABLE", - "release": {"display_name": "Google Calendar"} + "display_name": "Google Calendar" }, { "id": "plugin_github", "name": "github", - "status": "ENABLED", - "installation_policy": "AVAILABLE", - "release": {"display_name": "GitHub"} + "display_name": "GitHub" } ] })), @@ -740,8 +748,7 @@ async fn endpoint_mode_injects_candidates_hides_list_and_rejects_invented_ids() } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn endpoint_recommendation_adds_install_identity_only_to_elicitation_metadata() -> Result<()> -{ +async fn endpoint_recommendation_hydrates_install_identity_after_selection() -> Result<()> { skip_if_no_network!(Ok(())); run_remote_plugin_install_metadata_case().await @@ -760,16 +767,30 @@ async fn run_remote_plugin_install_metadata_case() -> Result<()> { "plugins": [{ "id": REMOTE_PLUGIN_ID, "name": "github", - "status": "ENABLED", - "installation_policy": "AVAILABLE", - "release": { - "display_name": "GitHub", - "app_ids": [APP_CONNECTOR_ID] - } + "display_name": "GitHub" }] })), ) .await; + Mock::given(method("GET")) + .and(path(format!("/ps/plugins/{REMOTE_PLUGIN_ID}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "id": REMOTE_PLUGIN_ID, + "name": "github", + "scope": "GLOBAL", + "status": "ENABLED", + "installation_policy": "AVAILABLE", + "authentication_policy": "ON_USE", + "release": { + "display_name": "GitHub", + "description": "Work with GitHub repositories.", + "app_ids": [APP_CONNECTOR_ID], + "interface": {} + } + }))) + .expect(1) + .mount(&server) + .await; let call_id = "install-github"; let mock = mount_sse_sequence( &server, @@ -863,6 +884,82 @@ async fn run_remote_plugin_install_metadata_case() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn endpoint_recommendation_skips_unavailable_plugin_elicitation() -> Result<()> { + skip_if_no_network!(Ok(())); + + const REMOTE_PLUGIN_ID: &str = "plugin_connector_github"; + + let server = start_mock_server().await; + let apps_server = AppsTestServer::mount(&server).await?; + mount_recommendations( + &server, + ResponseTemplate::new(200).set_body_json(json!({ + "enabled": true, + "plugins": [{ + "id": REMOTE_PLUGIN_ID, + "name": "github", + "display_name": "GitHub" + }] + })), + ) + .await; + Mock::given(method("GET")) + .and(path(format!("/ps/plugins/{REMOTE_PLUGIN_ID}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "id": REMOTE_PLUGIN_ID, + "name": "github", + "scope": "GLOBAL", + "status": "DISABLED_BY_ADMIN", + "installation_policy": "AVAILABLE", + "authentication_policy": "ON_USE", + "release": { + "display_name": "GitHub", + "description": "Work with GitHub repositories.", + "app_ids": ["connector_github"], + "interface": {} + } + }))) + .expect(1) + .mount(&server) + .await; + let call_id = "install-unavailable-github"; + let mock = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_function_call( + call_id, + REQUEST_PLUGIN_INSTALL_TOOL_NAME, + &serde_json::to_string(&json!({ + "plugin_id": "github@openai-curated-remote", + "suggest_reason": "Use GitHub for this request" + }))?, + ), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-1", "done"), + ev_completed("resp-2"), + ]), + ], + ) + .await; + let test = build_test(&server, &apps_server).await?; + + test.submit_turn("use GitHub").await?; + + let requests = mock.requests(); + assert_eq!(requests.len(), 2); + assert_eq!( + requests[1].function_call_output_text(call_id).as_deref(), + Some("The recommended plugins for this turn are no longer available.") + ); + Ok(()) +} + #[derive(Clone, Copy)] enum RefreshedAppsTools { Available, @@ -998,7 +1095,7 @@ async fn endpoint_mode_with_no_eligible_candidates_exposes_no_suggestion_tools() "plugins": [{ "id": "plugin_google_calendar", "name": "google-calendar", - "release": {"display_name": "Google Calendar"} + "display_name": "Google Calendar" }] })), )