diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index fbeab960fa..24cf0c9ba4 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -34,8 +34,6 @@ use crate::marketplace_upgrade::upgrade_configured_git_marketplaces; use crate::remote::RemoteInstalledPlugin; use crate::remote::RemotePluginCatalogError; use crate::remote::RemotePluginServiceConfig; -use crate::remote_bundle::download_and_install_remote_plugin_bundle; -use crate::remote_bundle::validate_remote_plugin_bundle; use crate::remote_legacy::RemotePluginFetchError; use crate::remote_legacy::RemotePluginMutationError; use crate::remote_startup_sync::RemoteStartupPluginSyncRequest; @@ -46,7 +44,6 @@ use crate::startup_sync::sync_openai_plugins_repo; use crate::store::PluginInstallResult as StorePluginInstallResult; use crate::store::PluginStore; use crate::store::PluginStoreError; -use crate::store::validate_plugin_version_segment; use codex_config::ConfigEditsBuilder; use codex_config::ConfigLayerStack; use codex_config::types::PluginConfig; @@ -1629,14 +1626,16 @@ impl PluginsManager { } }; - match self - .refresh_remote_installed_plugins_cache( - &request.service_config, - request.auth.as_ref(), - ) - .await - { - Ok(changed) => { + let installed_plugins = crate::remote::fetch_remote_installed_plugins( + &request.service_config, + request.auth.as_ref(), + ) + .await; + match installed_plugins { + Ok(installed_plugins) => { + // TODO(remote plugins): reconcile missing or stale local bundles before + // publishing remote installed state as effective local plugin config. + let changed = self.write_remote_installed_plugins_cache(installed_plugins); let should_notify = changed || matches!( request.notify, @@ -1671,167 +1670,6 @@ impl PluginsManager { } } - pub(crate) async fn refresh_remote_installed_plugins_cache( - &self, - service_config: &RemotePluginServiceConfig, - auth: Option<&CodexAuth>, - ) -> Result { - let installed_plugins = - crate::remote::fetch_remote_installed_plugins(service_config, auth).await?; - let previous_plugins_by_key = { - let cache = match self.remote_installed_plugins_cache.read() { - Ok(cache) => cache, - Err(err) => err.into_inner(), - }; - cache - .as_ref() - .map(|plugins| { - plugins - .iter() - .map(|plugin| { - ( - ( - plugin.marketplace_name.clone(), - plugin.id.clone(), - plugin.name.clone(), - ), - plugin.clone(), - ) - }) - .collect::>() - }) - .unwrap_or_default() - }; - let mut bundles_changed = false; - let mut publishable_plugins = Vec::new(); - for plugin in installed_plugins { - let previous_plugin = previous_plugins_by_key - .get(&( - plugin.marketplace_name.clone(), - plugin.id.clone(), - plugin.name.clone(), - )) - .cloned(); - let plugin_id = - match PluginId::new(plugin.name.clone(), plugin.marketplace_name.clone()) { - Ok(plugin_id) => plugin_id, - Err(err) => { - warn!( - remote_plugin_id = %plugin.id, - marketplace = %plugin.marketplace_name, - plugin = %plugin.name, - error = %err, - "ignoring remote installed plugin with invalid local plugin id" - ); - continue; - } - }; - - let release_version = plugin - .release_version - .as_deref() - .map(str::trim) - .filter(|version| !version.is_empty()); - - let Some(release_version) = release_version else { - if self.store.active_plugin_root(&plugin_id).is_some() { - publishable_plugins.push(plugin); - } else if let Some(mut previous_plugin) = previous_plugin { - previous_plugin.enabled = plugin.enabled; - publishable_plugins.push(previous_plugin); - } else { - warn!( - remote_plugin_id = %plugin.id, - marketplace = %plugin.marketplace_name, - plugin = %plugin.name, - "remote installed plugin is missing release metadata and no local bundle is available" - ); - } - continue; - }; - - if let Err(message) = validate_plugin_version_segment(release_version) { - if let Some(mut previous_plugin) = previous_plugin { - previous_plugin.enabled = plugin.enabled; - publishable_plugins.push(previous_plugin); - } - warn!( - remote_plugin_id = %plugin.id, - marketplace = %plugin.marketplace_name, - plugin = %plugin.name, - version = %release_version, - error = %message, - "ignoring remote installed plugin with invalid release version" - ); - continue; - } - - if self.store.active_plugin_version(&plugin_id).as_deref() == Some(release_version) { - publishable_plugins.push(plugin); - continue; - } - - let validated_bundle = match validate_remote_plugin_bundle( - &plugin.id, - &plugin.marketplace_name, - &plugin.name, - Some(release_version), - plugin.bundle_download_url.as_deref(), - ) { - Ok(bundle) => bundle, - Err(err) => { - if let Some(mut previous_plugin) = previous_plugin { - previous_plugin.enabled = plugin.enabled; - publishable_plugins.push(previous_plugin); - } - warn!( - remote_plugin_id = %plugin.id, - marketplace = %plugin.marketplace_name, - plugin = %plugin.name, - error = %err, - "failed to validate remote installed plugin bundle" - ); - continue; - } - }; - match download_and_install_remote_plugin_bundle( - self.codex_home.clone(), - validated_bundle, - ) - .await - { - Ok(result) => { - bundles_changed = true; - tracing::info!( - plugin = %result.plugin_id.as_key(), - version = %result.plugin_version, - path = %result.installed_path.display(), - "installed remote plugin bundle during installed-plugin refresh" - ); - publishable_plugins.push(plugin); - } - Err(err) => { - if let Some(mut previous_plugin) = previous_plugin { - previous_plugin.enabled = plugin.enabled; - publishable_plugins.push(previous_plugin); - } - warn!( - remote_plugin_id = %plugin.id, - marketplace = %plugin.marketplace_name, - plugin = %plugin.name, - error = %err, - "failed to install remote plugin bundle during installed-plugin refresh" - ); - } - } - } - let cache_changed = self.write_remote_installed_plugins_cache(publishable_plugins); - if bundles_changed { - self.clear_enabled_outcome_cache(); - } - Ok(cache_changed || bundles_changed) - } - fn run_non_curated_plugin_cache_refresh_loop(self: Arc) { loop { let request = { diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index 53de77f44b..88d43664a2 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -739,107 +739,6 @@ remote_plugin = true assert_eq!(outcome.plugins()[0].config_name, "linear@chatgpt-global"); } -async fn mount_remote_installed_plugin_pages( - server: &MockServer, - global_plugins: &str, - workspace_plugins: &str, -) { - let body_for_plugins = |plugins: &str| { - format!( - r#"{{ - "plugins": [ -{plugins} - ], - "pagination": {{ - "limit": 50, - "next_page_token": null - }} -}}"# - ) - }; - - Mock::given(method("GET")) - .and(path("/backend-api/ps/plugins/installed")) - .and(query_param("scope", "GLOBAL")) - .and(query_param("includeDownloadUrls", "true")) - .and(header("authorization", "Bearer Access Token")) - .and(header("chatgpt-account-id", "account_id")) - .respond_with(ResponseTemplate::new(200).set_body_string(body_for_plugins(global_plugins))) - .mount(server) - .await; - Mock::given(method("GET")) - .and(path("/backend-api/ps/plugins/installed")) - .and(query_param("scope", "WORKSPACE")) - .and(query_param("includeDownloadUrls", "true")) - .and(header("authorization", "Bearer Access Token")) - .and(header("chatgpt-account-id", "account_id")) - .respond_with( - ResponseTemplate::new(200).set_body_string(body_for_plugins(workspace_plugins)), - ) - .mount(server) - .await; -} - -fn remote_installed_plugin_json(plugin_name: &str, enabled: bool) -> String { - format!( - r#"{{ - "id": "plugins~Plugin_{plugin_name}", - "name": "{plugin_name}", - "scope": "GLOBAL", - "installation_policy": "AVAILABLE", - "authentication_policy": "ON_USE", - "release": {{ - "version": "local", - "bundle_download_url": "https://example.com/{plugin_name}.tar.gz", - "display_name": "{plugin_name}", - "description": "{plugin_name} plugin", - "app_ids": [], - "interface": {{ - "short_description": "{plugin_name}", - "capabilities": [] - }}, - "skills": [] - }}, - "enabled": {enabled}, - "disabled_skill_names": [] - }}"# - ) -} - -fn remote_installed_plugin_json_with_release( - plugin_name: &str, - enabled: bool, - version: &str, - bundle_download_url: Option<&str>, -) -> String { - let bundle_download_url = bundle_download_url - .map(|url| format!(r#""bundle_download_url": "{url}","#)) - .unwrap_or_default(); - format!( - r#"{{ - "id": "plugins~Plugin_{plugin_name}", - "name": "{plugin_name}", - "scope": "GLOBAL", - "installation_policy": "AVAILABLE", - "authentication_policy": "ON_USE", - "release": {{ - "version": "{version}", - {bundle_download_url} - "display_name": "{plugin_name}", - "description": "{plugin_name} plugin", - "app_ids": [], - "interface": {{ - "short_description": "{plugin_name}", - "capabilities": [] - }}, - "skills": [] - }}, - "enabled": {enabled}, - "disabled_skill_names": [] - }}"# - ) -} - async fn wait_for_counter(counter: &AtomicUsize, expected: usize) { tokio::time::timeout(Duration::from_secs(5), async { loop { @@ -853,177 +752,6 @@ async fn wait_for_counter(counter: &AtomicUsize, expected: usize) { .expect("counter should reach expected value"); } -#[tokio::test] -async fn remote_installed_plugins_cache_refresh_does_not_publish_stale_plugin_when_bundle_unavailable() - { - let tmp = tempfile::tempdir().unwrap(); - write_plugin( - &tmp.path().join("plugins/cache/chatgpt-global"), - "linear/1.0.0", - "linear", - ); - write_file( - &tmp.path().join(CONFIG_TOML_FILE), - r#"[features] -plugins = true -remote_plugin = true -"#, - ); - - let server = MockServer::start().await; - mount_remote_installed_plugin_pages( - &server, - &remote_installed_plugin_json_with_release( - "linear", /*enabled*/ true, "2.0.0", /*bundle_download_url*/ None, - ), - "", - ) - .await; - - let config = load_config(tmp.path(), tmp.path()).await; - let manager = PluginsManager::new(tmp.path().to_path_buf()); - let changed = manager - .refresh_remote_installed_plugins_cache( - &remote_plugin_service_config(&format!("{}/backend-api/", server.uri())), - Some(&CodexAuth::create_dummy_chatgpt_auth_for_testing()), - ) - .await - .unwrap(); - - assert!(changed); - let outcome = manager.plugins_for_test_config(&config).await; - assert_eq!(outcome, PluginLoadOutcome::default()); -} - -#[tokio::test] -async fn remote_installed_plugins_cache_refresh_preserves_previous_bundle_when_upgrade_unavailable() -{ - let tmp = tempfile::tempdir().unwrap(); - let linear_root = tmp.path().join("plugins/cache/chatgpt-global/linear/1.0.0"); - write_plugin( - &tmp.path().join("plugins/cache/chatgpt-global"), - "linear/1.0.0", - "linear", - ); - write_file( - &tmp.path().join(CONFIG_TOML_FILE), - r#"[features] -plugins = true -remote_plugin = true -"#, - ); - - let server = MockServer::start().await; - mount_remote_installed_plugin_pages( - &server, - &remote_installed_plugin_json_with_release( - "linear", /*enabled*/ true, "2.0.0", /*bundle_download_url*/ None, - ), - "", - ) - .await; - - let config = load_config(tmp.path(), tmp.path()).await; - let manager = PluginsManager::new(tmp.path().to_path_buf()); - manager.write_remote_installed_plugins_cache(vec![RemoteInstalledPlugin { - marketplace_name: "chatgpt-global".to_string(), - id: "plugins~Plugin_linear".to_string(), - name: "linear".to_string(), - enabled: true, - release_version: Some("1.0.0".to_string()), - bundle_download_url: Some("https://example.com/linear-1.0.0.tar.gz".to_string()), - }]); - let changed = manager - .refresh_remote_installed_plugins_cache( - &remote_plugin_service_config(&format!("{}/backend-api/", server.uri())), - Some(&CodexAuth::create_dummy_chatgpt_auth_for_testing()), - ) - .await - .unwrap(); - - assert!(!changed); - let outcome = manager.plugins_for_test_config(&config).await; - assert_eq!( - outcome.effective_skill_roots(), - vec![AbsolutePathBuf::try_from(linear_root.join("skills")).unwrap()] - ); - assert_eq!(outcome.plugins().len(), 1); - assert_eq!(outcome.plugins()[0].config_name, "linear@chatgpt-global"); -} - -#[tokio::test] -async fn remote_installed_plugins_cache_refresh_reconciles_cached_bundles_without_config_writes() { - let tmp = tempfile::tempdir().unwrap(); - let linear_root = tmp.path().join("plugins/cache/chatgpt-global/linear/local"); - write_plugin( - &tmp.path().join("plugins/cache/chatgpt-global"), - "linear/local", - "linear", - ); - write_plugin( - &tmp.path().join("plugins/cache/chatgpt-global"), - "gmail/local", - "gmail", - ); - write_plugin( - &tmp.path().join("plugins/cache/chatgpt-global"), - "calendar/local", - "calendar", - ); - let config_toml = r#"[features] -plugins = true -remote_plugin = true -"#; - write_file(&tmp.path().join(CONFIG_TOML_FILE), config_toml); - - let server = MockServer::start().await; - mount_remote_installed_plugin_pages( - &server, - &format!( - "{},\n{}", - remote_installed_plugin_json("linear", /*enabled*/ true), - remote_installed_plugin_json("gmail", /*enabled*/ false) - ), - "", - ) - .await; - - let config = load_config(tmp.path(), tmp.path()).await; - let manager = PluginsManager::new(tmp.path().to_path_buf()); - let changed = manager - .refresh_remote_installed_plugins_cache( - &remote_plugin_service_config(&format!("{}/backend-api/", server.uri())), - Some(&CodexAuth::create_dummy_chatgpt_auth_for_testing()), - ) - .await - .unwrap(); - - assert!(changed); - assert_eq!( - fs::read_to_string(tmp.path().join(CONFIG_TOML_FILE)).unwrap(), - config_toml - ); - - let outcome = manager.plugins_for_test_config(&config).await; - let mut plugin_states = outcome - .plugins() - .iter() - .map(|plugin| (plugin.config_name.clone(), plugin.enabled)) - .collect::>(); - plugin_states.sort(); - assert_eq!( - plugin_states, - vec![ - ("gmail@chatgpt-global".to_string(), false), - ("linear@chatgpt-global".to_string(), true), - ] - ); - assert_eq!( - outcome.effective_skill_roots(), - vec![AbsolutePathBuf::try_from(linear_root.join("skills")).unwrap()] - ); -} - #[tokio::test] async fn remote_installed_plugins_cache_refresh_does_not_start_when_feature_disabled() { let tmp = tempfile::tempdir().unwrap();