From 0c9be8a836a65681bb4e2366f05590babf89edf2 Mon Sep 17 00:00:00 2001 From: vkg-oai Date: Thu, 17 Sep 2026 21:27:59 +0000 Subject: [PATCH] Preserve plugin caches across display metadata refreshes (#46309) ## Why Renewed image URLs and other display metadata changes unnecessarily invalidate loaded plugins and MCP and skill caches, even when installed plugin behavior is unchanged. ## What changed - Compare installed plugin metadata by identity, version, enablement, policy, and availability before invalidating derived caches. Continue storing the full updated payload so display consumers receive fresh metadata. - Preserve invalidation when behavioral metadata changes or reconciliation requires an effective plugin refresh. - Export `remote_catalog_metadata_eq` to compare catalogs independently of display metadata and plugin display order, while retaining marketplace order significance. ## Testing Add regression tests for display-only updates, behavioral changes, catalog ordering, and preservation of loaded skills and tool suggestions. Add an app-server integration test verifying that image URL renewals and badge updates preserve live MCP sessions and cached skill resources, while an authentication policy change invalidates resource caches. GitOrigin-RevId: 22b9ba1234a9f850201c6890e03d1ef899a56c54 --- .../app-server/tests/suite/v2/mcp_resource.rs | 3 + .../mcp_resource/plugin_metadata_refresh.rs | 284 ++++++++++++++++++ codex-rs/core-plugins/src/lib.rs | 2 + codex-rs/core-plugins/src/manager.rs | 10 +- codex-rs/core-plugins/src/manager_tests.rs | 8 +- codex-rs/core-plugins/src/remote_metadata.rs | 77 +++++ .../src/remote_metadata_cache_tests.rs | 135 +++++++++ .../core-plugins/src/remote_metadata_tests.rs | 181 +++++++++++ 8 files changed, 697 insertions(+), 3 deletions(-) create mode 100644 codex-rs/app-server/tests/suite/v2/mcp_resource/plugin_metadata_refresh.rs create mode 100644 codex-rs/core-plugins/src/remote_metadata.rs create mode 100644 codex-rs/core-plugins/src/remote_metadata_cache_tests.rs create mode 100644 codex-rs/core-plugins/src/remote_metadata_tests.rs diff --git a/codex-rs/app-server/tests/suite/v2/mcp_resource.rs b/codex-rs/app-server/tests/suite/v2/mcp_resource.rs index 60d4a65585..426e5b051c 100644 --- a/codex-rs/app-server/tests/suite/v2/mcp_resource.rs +++ b/codex-rs/app-server/tests/suite/v2/mcp_resource.rs @@ -117,6 +117,9 @@ use tokio::sync::Notify; use tokio::task::JoinHandle; use tokio::time::timeout; +#[path = "mcp_resource/plugin_metadata_refresh.rs"] +mod plugin_metadata_refresh; + pub(super) const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(10); const TEST_RESOURCE_URI: &str = "test://codex/resource"; pub(super) const TEST_WIDGET_RESOURCE_URI: &str = "ui://widget/checkout-session.html"; diff --git a/codex-rs/app-server/tests/suite/v2/mcp_resource/plugin_metadata_refresh.rs b/codex-rs/app-server/tests/suite/v2/mcp_resource/plugin_metadata_refresh.rs new file mode 100644 index 0000000000..ee8b4d52c0 --- /dev/null +++ b/codex-rs/app-server/tests/suite/v2/mcp_resource/plugin_metadata_refresh.rs @@ -0,0 +1,284 @@ +//! Exercises installed-metadata refreshes against live MCP and skill caches. + +use super::*; +use axum::Json; +use axum::routing::get; +use codex_app_server_protocol::PluginInstalledResponse; +use codex_app_server_protocol::PluginReconcileResponse; +use flate2::Compression; +use flate2::write::GzEncoder; +use pretty_assertions::assert_eq; +use tokio::sync::RwLock; + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn signed_image_renewal_preserves_live_mcp_and_skills() -> Result<()> { + let responses_server = responses::start_mock_server().await; + let listener = TcpListener::bind("127.0.0.1:0").await?; + let base_url = format!("http://{}", listener.local_addr()?); + let original_logo = "https://files.openai.com/plugins/logo.png?sv=1&sr=b&sig=first&se=old"; + let renewed_logo = "https://files.openai.com/plugins/logo.png?sv=1&sr=b&sig=second&se=new"; + let changed_logo = "https://files.openai.com/plugins/new-logo.png?sv=1&sr=b&sig=third&se=new"; + let installed = Arc::new(RwLock::new(json!({ + "id": "plugins~Plugin_00000000000000000000000000000000", + "name": "demo-plugin", + "scope": "GLOBAL", + "installation_policy": "AVAILABLE", + "authentication_policy": "ON_USE", + "release": { + "version": "1.0.0", + "display_name": "Demo plugin", + "description": "Test plugin", + "bundle_download_url": format!("{base_url}/bundle"), + "interface": {"logo_url": original_logo}, + }, + "enabled": true, + }))); + let manifest = br#"{"name":"demo-plugin"}"#; + let mut archive = tar::Builder::new(GzEncoder::new(Vec::new(), Compression::default())); + let mut header = tar::Header::new_gnu(); + header.set_size(manifest.len() as u64); + header.set_mode(/*mode*/ 0o644); + header.set_cksum(); + archive.append_data(&mut header, ".codex-plugin/plugin.json", &manifest[..])?; + let bundle = archive.into_inner()?.finish()?; + + let calls = Arc::new(ResourceAppsMcpCalls::default()); + let sessions = Arc::new(AtomicUsize::new(0)); + let server_calls = Arc::clone(&calls); + let server_sessions = Arc::clone(&sessions); + let mcp_service = StreamableHttpService::new( + move || { + server_sessions.fetch_add(1, Ordering::SeqCst); + Ok(MetadataMcpServer(ResourceAppsMcpServer { + calls: Arc::clone(&server_calls), + })) + }, + Arc::new(LocalSessionManager::default()), + StreamableHttpServerConfig::default(), + ); + let server_installed = Arc::clone(&installed); + let router = Router::new() + .nest_service("/api/codex/ps/mcp", mcp_service) + .route( + "/ps/plugins/installed", + get(move || { + let installed = Arc::clone(&server_installed); + async move { + Json(json!({ + "plugins": [installed.read().await.clone()], + "pagination": {"limit": 200, "next_page_token": null}, + })) + } + }), + ) + .route("/bundle", get(move || async move { bundle })); + let server_handle = tokio::spawn(async move { + let _ = axum::serve(listener, router).await; + }); + + let codex_home = TempDir::new()?; + MockResponsesConfig::new(&responses_server.uri()) + .with_root_config(&format!("chatgpt_base_url = \"{base_url}\"")) + .enable_feature(Feature::Apps) + .enable_feature(Feature::Plugins) + .enable_feature(Feature::RemotePlugin) + .disable_feature(Feature::PluginSharing) + .with_extra_config("[skills]\ninclude_instructions = true") + .write(codex_home.path())?; + write_chatgpt_auth( + codex_home.path(), + ChatGptAuthFixture::new("chatgpt-token") + .account_id("account-123") + .chatgpt_user_id("user-123") + .chatgpt_account_id("account-123"), + AuthCredentialsStoreMode::File, + )?; + let mut app_server = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_managed_config() + .with_env_overrides(&[( + "CODEX_TEST_ALLOW_HTTP_REMOTE_PLUGIN_BUNDLE_DOWNLOADS", + Some("1"), + )]) + .build_initialized() + .await?; + refresh_and_expect_logo(&mut app_server, original_logo).await?; + // Hosted skills are exposed without a local executor, as in the other resource tests. + let request_id = app_server + .send_thread_start_request(ThreadStartParams { + model: Some("gpt-5.5".to_string()), + environments: Some(Vec::new()), + ..Default::default() + }) + .await?; + let ThreadStartResponse { thread, .. } = + timeout(DEFAULT_READ_TIMEOUT, app_server.read_response(request_id)).await??; + + read_skill_in_turn(&mut app_server, &responses_server, &thread.id).await?; + let warm_sessions = sessions.load(Ordering::SeqCst); + assert!(warm_sessions > 0); + let warm_calls = calls.snapshot(); + assert_eq!(warm_calls.list_resources, 1); + assert_eq!(warm_calls.main_prompt_reads, 1); + + installed.write().await["release"]["interface"]["logo_url"] = json!(renewed_logo); + refresh_and_expect_logo(&mut app_server, renewed_logo).await?; + read_skill_in_turn(&mut app_server, &responses_server, &thread.id).await?; + assert_eq!(sessions.load(Ordering::SeqCst), warm_sessions); + assert_eq!(calls.snapshot(), warm_calls); + + installed.write().await["release"]["interface"]["capabilities"] = + json!(["Updated store badge"]); + refresh_and_expect_logo(&mut app_server, renewed_logo).await?; + read_skill_in_turn(&mut app_server, &responses_server, &thread.id).await?; + assert_eq!(sessions.load(Ordering::SeqCst), warm_sessions); + assert_eq!(calls.snapshot(), warm_calls); + + // A policy change must invalidate both caches without rewriting any bundle files. + installed.write().await["release"]["interface"]["logo_url"] = json!(changed_logo); + installed.write().await["authentication_policy"] = json!("ON_INSTALL"); + refresh_and_expect_logo(&mut app_server, changed_logo).await?; + read_skill_in_turn(&mut app_server, &responses_server, &thread.id).await?; + // Core may retain the HTTP connection, but the callback must invalidate resources. + assert_eq!( + calls.snapshot(), + ResourceAppsMcpCallCounts { + list_resources: 2, + main_prompt_reads: 2, + reference_reads: 0, + } + ); + server_handle.abort(); + Ok(()) +} + +async fn refresh_and_expect_logo(app_server: &mut TestAppServer, logo: &str) -> Result<()> { + // plugin/installed starts the real background sync with the production callback. + // Reconcile acquires the same gate, so its completion fences the background pass. + let request = app_server + .send_raw_request("plugin/installed", Some(json!({}))) + .await?; + let _: PluginInstalledResponse = + timeout(DEFAULT_READ_TIMEOUT, app_server.read_response(request)).await??; + let request = app_server + .send_raw_request("plugin/reconcile", Some(json!({}))) + .await?; + let reconciled: PluginReconcileResponse = + timeout(DEFAULT_READ_TIMEOUT, app_server.read_response(request)).await??; + assert!(reconciled.failed_remote_plugin_ids.is_empty()); + assert!( + reconciled + .failed_materialization_remote_plugin_ids + .is_empty() + ); + let request = app_server + .send_raw_request("plugin/installed", Some(json!({}))) + .await?; + let installed: PluginInstalledResponse = + timeout(DEFAULT_READ_TIMEOUT, app_server.read_response(request)).await??; + let plugin = installed + .marketplaces + .iter() + .flat_map(|marketplace| &marketplace.plugins) + .find(|plugin| plugin.name == "demo-plugin") + .context("installed plugin should be exposed through the public API")?; + assert_eq!( + plugin + .interface + .as_ref() + .and_then(|info| info.logo_url.as_deref()), + Some(logo) + ); + // Finish the follow-up read's background pass before changing the next snapshot. + let request = app_server + .send_raw_request("plugin/reconcile", Some(json!({}))) + .await?; + let _: PluginReconcileResponse = + timeout(DEFAULT_READ_TIMEOUT, app_server.read_response(request)).await??; + Ok(()) +} + +async fn read_skill_in_turn( + app_server: &mut TestAppServer, + responses_server: &wiremock::MockServer, + thread_id: &str, +) -> Result<()> { + let response_mock = responses::mount_sse_sequence( + responses_server, + vec![ + responses::sse(vec![ + responses::ev_function_call_with_namespace( + "read-main", + "skills", + "read", + &json!({"package": SKILL_RESOURCE_URI}).to_string(), + ), + responses::ev_completed("read"), + ]), + responses::sse(vec![responses::ev_completed("done")]), + ], + ) + .await; + let completed = timeout( + DEFAULT_READ_TIMEOUT, + app_server.start_turn_and_wait_for_completion(TurnStartParams { + thread_id: thread_id.to_string(), + input: vec![UserInput::Text { + text: "Read the deployment skill.".to_string(), + text_elements: Vec::new(), + }], + ..Default::default() + }), + ) + .await??; + assert_eq!(completed.turn.status, TurnStatus::Completed); + let requests = response_mock.requests(); + assert_eq!(requests.len(), 2); + assert!( + requests[0] + .message_input_texts("developer") + .iter() + .any(|text| text.contains(SKILL_NAME)) + ); + let output = requests[1] + .function_call_output_text("read-main") + .context("skill read should reach the model")?; + assert_eq!( + serde_json::from_str::(&output)?, + json!({"resource": SKILL_MAIN_PROMPT_URI, "contents": SKILL_CONTENTS, "next_cursor": null}) + ); + Ok(()) +} + +// Reuse the resource-read fixture with a healthy, single-page skills catalog. +struct MetadataMcpServer(ResourceAppsMcpServer); + +impl ServerHandler for MetadataMcpServer { + fn get_info(&self) -> ServerInfo { + self.0.get_info() + } + + async fn list_resources( + &self, + _request: Option, + _context: RequestContext, + ) -> Result { + self.0.calls.list_resources.fetch_add(1, Ordering::Relaxed); + Ok(ListResourcesResult::with_all_items(vec![skill_resource( + SKILL_RESOURCE_URI, + "plugin_demo/deploy", + RAW_SKILL_DESCRIPTION, + "mcp/skill", + "demo-plugin", + "deploy", + )])) + } + + async fn read_resource( + &self, + request: ReadResourceRequestParams, + context: RequestContext, + ) -> Result { + self.0.read_resource(request, context).await + } +} diff --git a/codex-rs/core-plugins/src/lib.rs b/codex-rs/core-plugins/src/lib.rs index ed440b5d97..20b6fc0e47 100644 --- a/codex-rs/core-plugins/src/lib.rs +++ b/codex-rs/core-plugins/src/lib.rs @@ -25,6 +25,7 @@ mod recommended_plugin_install; pub mod remote; pub mod remote_bundle; pub mod remote_legacy; +mod remote_metadata; mod remote_plugin_id_resolver; mod script_attribution; mod skill_snapshots; @@ -105,6 +106,7 @@ 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 remote_metadata::remote_catalog_metadata_eq; pub use script_attribution::PluginCommandAttribution; pub use script_attribution::TrustedPluginRoots; pub use script_attribution::command_script_arguments; diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index cce218323e..c90794ee9d 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -1417,10 +1417,16 @@ impl PluginsManager { } return Some(needs_effective_plugins_refresh); } + let metadata_changed = cache.plugins.as_ref().is_none_or(|previous| { + !crate::remote_metadata::installed_plugin_metadata_eq(previous, &plugins) + }); cache.plugins = Some(plugins); drop(cache); - self.clear_loaded_plugins_cache(); - Some(true) + let changed = needs_effective_plugins_refresh || metadata_changed; + if changed { + self.clear_loaded_plugins_cache(); + } + Some(changed) } #[cfg(test)] diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index c92c4fc687..17e6271d40 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -78,6 +78,9 @@ const MAX_CAPABILITY_SUMMARY_DESCRIPTION_LEN: usize = 1024; #[path = "marketplace_policy/curated_loading_tests.rs"] mod curated_marketplace_policy; +#[path = "remote_metadata_cache_tests.rs"] +mod remote_metadata_cache; + fn unrestricted_config_layer_stack() -> ConfigLayerStack { ConfigLayerStack::default() } @@ -7270,7 +7273,10 @@ remote_plugin = true /*on_effective_plugins_changed*/ None, ); - tokio::time::sleep(Duration::from_millis(400)).await; + let _guard = first_manager + .acquire_remote_installed_plugin_sync_guard() + .await + .expect("background bundle sync should finish"); server.verify().await; } diff --git a/codex-rs/core-plugins/src/remote_metadata.rs b/codex-rs/core-plugins/src/remote_metadata.rs new file mode 100644 index 0000000000..a4db4055c1 --- /dev/null +++ b/codex-rs/core-plugins/src/remote_metadata.rs @@ -0,0 +1,77 @@ +//! Compares the metadata that determines installed-plugin behavior. +//! +//! Display metadata (including image URLs and store capability badges) is excluded. Callers must +//! still publish the unmodified payload so display consumers receive fresh metadata. + +use crate::remote::RemoteInstalledPlugin; +use crate::remote::RemoteMarketplace; + +pub(crate) fn installed_plugin_metadata_eq( + previous: &[RemoteInstalledPlugin], + current: &[RemoteInstalledPlugin], +) -> bool { + previous.len() == current.len() + && previous.iter().zip(current).all(|(previous, current)| { + previous.marketplace_name == current.marketplace_name + && previous.id == current.id + && previous.name == current.name + && previous.canonical_app_id == current.canonical_app_id + && previous.version == current.version + && previous.installed_at == current.installed_at + && previous.enabled == current.enabled + && previous.install_policy == current.install_policy + && previous.install_policy_source == current.install_policy_source + && previous.must_show_installation_interstitial + == current.must_show_installation_interstitial + && previous.auth_policy == current.auth_policy + && previous.availability == current.availability + && previous.disabled_reason == current.disabled_reason + && previous.eligible_plan_types == current.eligible_plan_types + }) +} + +/// Compare installed catalog identity, versions, enablement, policy and availability. +/// +/// Plugin display order is ignored. Marketplace ordering remains significant. +/// Callers must still store the unmodified current catalog for display consumers. +pub fn remote_catalog_metadata_eq( + previous: &[RemoteMarketplace], + current: &[RemoteMarketplace], +) -> bool { + previous.len() == current.len() + && previous.iter().zip(current).all(|(previous, current)| { + if previous.name != current.name || previous.plugins.len() != current.plugins.len() { + return false; + } + // Core sorts this view by display name, which can change independently of behavior. + let mut previous = previous.plugins.iter().collect::>(); + let mut current = current.plugins.iter().collect::>(); + previous.sort_unstable_by(|a, b| a.remote_plugin_id.cmp(&b.remote_plugin_id)); + current.sort_unstable_by(|a, b| a.remote_plugin_id.cmp(&b.remote_plugin_id)); + previous + .into_iter() + .zip(current) + .all(|(previous, current)| { + previous.id == current.id + && previous.remote_plugin_id == current.remote_plugin_id + && previous.name == current.name + && previous.version == current.version + && previous.local_version == current.local_version + && previous.installed == current.installed + && previous.installed_at == current.installed_at + && previous.enabled == current.enabled + && previous.install_policy == current.install_policy + && previous.install_policy_source == current.install_policy_source + && previous.must_show_installation_interstitial + == current.must_show_installation_interstitial + && previous.auth_policy == current.auth_policy + && previous.availability == current.availability + && previous.disabled_reason == current.disabled_reason + && previous.eligible_plan_types == current.eligible_plan_types + }) + }) +} + +#[cfg(test)] +#[path = "remote_metadata_tests.rs"] +mod tests; diff --git a/codex-rs/core-plugins/src/remote_metadata_cache_tests.rs b/codex-rs/core-plugins/src/remote_metadata_cache_tests.rs new file mode 100644 index 0000000000..c93023c8bf --- /dev/null +++ b/codex-rs/core-plugins/src/remote_metadata_cache_tests.rs @@ -0,0 +1,135 @@ +//! Metadata publication must preserve derived caches until runtime inputs change. + +use super::*; +use pretty_assertions::assert_eq; + +#[tokio::test] +async fn display_refresh_preserves_loaded_skills_and_tool_suggestions() { + let codex_home = TempDir::new().unwrap(); + let marketplace_root = codex_home + .path() + .join("plugins/cache/openai-curated-remote"); + write_plugin(&marketplace_root, "sample/local", "sample"); + let plugin_root = marketplace_root.join("sample/local"); + write_file( + &codex_home.path().join(CONFIG_TOML_FILE), + r#"[features] +plugins = true +remote_plugin = true + +[plugins."sample@openai-curated-remote"] +enabled = true +"#, + ); + let config = load_config(codex_home.path(), codex_home.path()).await; + let manager = test_plugins_manager_with_options( + codex_home.path().to_path_buf(), + Some(Product::Codex), + Some(AuthMode::Chatgpt), + ); + let mut remote = remote_installed_plugin("sample"); + remote.interface = Some( + serde_json::from_value(serde_json::json!({ + "capabilities": [], "screenshots": [], "screenshotUrls": [], + "logoUrl": "https://files.openai.com/logo.png?sig=old" + })) + .unwrap(), + ); + assert!(manager.write_remote_installed_plugins_cache(vec![remote.clone()])); + + let loaded = manager.plugins_for_config(&config).await; + let snapshots = manager.plugin_skill_snapshots_for_config(&config).unwrap(); + let roots = loaded.effective_plugin_skill_roots(); + assert_eq!(roots.len(), 1); + assert_eq!(snapshots.get(&roots[0]).unwrap().skills.len(), 1); + let loaded_generation = manager.loaded_plugins_cache_generation(); + let plugin = ConfiguredMarketplacePlugin { + id: "sample@openai-curated-remote".to_string(), + name: "sample".to_string(), + local_version: None, + installed_version: None, + source: MarketplacePluginSource::Local { + path: plugin_root.abs(), + }, + policy: MarketplacePluginPolicy { + installation: MarketplacePluginInstallPolicy::Available, + authentication: MarketplacePluginAuthPolicy::OnUse, + products: None, + }, + interface: None, + keywords: Vec::new(), + manifest_fallback: None, + installed: true, + enabled: true, + }; + let suggestions = manager + .tool_suggest_metadata_cache + .metadata_for_plugin( + REMOTE_GLOBAL_MARKETPLACE_NAME, + &plugin, + manager.restriction_product, + manager.skill_root_loader.as_ref(), + ) + .await + .unwrap(); + assert!( + suggestions + .project(&SkillConfigRules::default(), manager.auth_mode()) + .has_skills + ); + + let interface = remote.interface.as_mut().unwrap(); + interface.logo_url = Some("https://files.openai.com/logo.png?sig=new".to_string()); + interface.display_name = Some("Updated display name".to_string()); + interface.capabilities = vec!["Updated store badge".to_string()]; + assert!(!manager.write_remote_installed_plugins_cache(vec![remote.clone()])); + assert_eq!( + manager + .remote_installed_plugins_cache + .read() + .unwrap() + .plugins, + Some(vec![remote.clone()]) + ); + assert_eq!(manager.loaded_plugins_cache_generation(), loaded_generation); + assert_eq!( + manager.plugin_skill_snapshots_for_config(&config), + Some(snapshots.clone()) + ); + assert_eq!(manager.plugins_for_config(&config).await, loaded); + let refreshed_suggestions = manager + .tool_suggest_metadata_cache + .metadata_for_plugin( + REMOTE_GLOBAL_MARKETPLACE_NAME, + &plugin, + manager.restriction_product, + manager.skill_root_loader.as_ref(), + ) + .await + .unwrap(); + assert!(Arc::ptr_eq(&suggestions, &refreshed_suggestions)); + + remote.version = Some("2.0.0".to_string()); + assert!(manager.write_remote_installed_plugins_cache(vec![remote])); + assert_eq!( + manager.loaded_plugins_cache_generation(), + loaded_generation + 1 + ); + assert_eq!(manager.plugin_skill_snapshots_for_config(&config), None); + manager.plugins_for_config(&config).await; + assert_ne!( + manager.plugin_skill_snapshots_for_config(&config), + Some(snapshots) + ); + let changed_suggestions = manager + .tool_suggest_metadata_cache + .metadata_for_plugin( + REMOTE_GLOBAL_MARKETPLACE_NAME, + &plugin, + manager.restriction_product, + manager.skill_root_loader.as_ref(), + ) + .await + .unwrap(); + assert!(!Arc::ptr_eq(&suggestions, &changed_suggestions)); +} diff --git a/codex-rs/core-plugins/src/remote_metadata_tests.rs b/codex-rs/core-plugins/src/remote_metadata_tests.rs new file mode 100644 index 0000000000..3e61f3a2ea --- /dev/null +++ b/codex-rs/core-plugins/src/remote_metadata_tests.rs @@ -0,0 +1,181 @@ +//! Regression coverage for the fields that affect plugin behavior. + +use super::*; +use crate::remote::REMOTE_GLOBAL_MARKETPLACE_NAME; +use crate::remote::group_remote_installed_plugins_by_marketplaces; +use codex_app_server_protocol::PluginAuthPolicy; +use codex_app_server_protocol::PluginAvailability; +use codex_app_server_protocol::PluginInstallPolicy; +use serde_json::json; + +fn image_url(renewal: &str) -> String { + format!("https://files.openai.com/plugins/icon.png?sig={renewal}") +} + +fn plugin(renewal: &str) -> RemoteInstalledPlugin { + RemoteInstalledPlugin { + marketplace_name: REMOTE_GLOBAL_MARKETPLACE_NAME.to_string(), + id: "plugin-test".to_string(), + version: Some("1.0.0".to_string()), + name: "test".to_string(), + canonical_app_id: None, + installed_at: None, + enabled: true, + install_policy: PluginInstallPolicy::Available, + install_policy_source: None, + must_show_installation_interstitial: None, + auth_policy: PluginAuthPolicy::OnUse, + availability: PluginAvailability::Available, + disabled_reason: None, + eligible_plan_types: None, + interface: Some( + serde_json::from_value(json!({ + "capabilities": [], "screenshots": [], + "logoUrl": image_url(renewal), "logoUrlDark": image_url(renewal), + "composerIconUrl": image_url(renewal), "screenshotUrls": [image_url(renewal)] + })) + .unwrap(), + ), + keywords: vec![], + } +} + +fn catalog(plugins: &[RemoteInstalledPlugin]) -> Vec { + group_remote_installed_plugins_by_marketplaces(plugins, &[REMOTE_GLOBAL_MARKETPLACE_NAME]) +} + +#[test] +fn behavioral_metadata_changes_still_invalidate() { + let changes: &[fn(&mut RemoteInstalledPlugin)] = &[ + |plugin| plugin.id.push_str("-other"), + |plugin| plugin.name.push_str("-other"), + |plugin| plugin.version = Some("2.0.0".to_string()), + |plugin| { + plugin.installed_at = + chrono::DateTime::from_timestamp(/*secs*/ 1, /*nsecs*/ 0) + }, + |plugin| plugin.enabled = false, + |plugin| plugin.install_policy = PluginInstallPolicy::NotAvailable, + |plugin| { + plugin.install_policy_source = + Some(codex_app_server_protocol::PluginInstallPolicySource::WorkspaceSetting) + }, + |plugin| plugin.must_show_installation_interstitial = Some(true), + |plugin| plugin.auth_policy = PluginAuthPolicy::OnInstall, + |plugin| plugin.availability = PluginAvailability::DisabledByAdmin, + |plugin| plugin.eligible_plan_types = Some(vec!["enterprise".to_string()]), + ]; + for change in changes { + let previous = [plugin("old")]; + let mut current = [plugin("new")]; + change(&mut current[0]); + assert!(!installed_plugin_metadata_eq(&previous, ¤t)); + assert!(!remote_catalog_metadata_eq( + &catalog(&previous), + &catalog(¤t) + )); + } + let previous = [plugin("old")]; + let mut current = [plugin("new")]; + current[0].canonical_app_id = Some("new-app".to_string()); + assert!(!installed_plugin_metadata_eq(&previous, ¤t)); + assert!(!installed_plugin_metadata_eq(&previous, &[])); + assert!(!remote_catalog_metadata_eq(&catalog(&previous), &[])); +} + +#[test] +fn display_metadata_does_not_invalidate() { + let previous = [plugin("old")]; + let mut current = [plugin("new")]; + current[0].keywords.push("new keyword".to_string()); + let interface = current[0].interface.as_mut().unwrap(); + interface.logo_url = Some("https://another-host.test/different-image.svg".to_string()); + interface.display_name = Some("New display name".to_string()); + interface.short_description = Some("New description".to_string()); + interface.brand_color = Some("#123456".to_string()); + interface.default_prompt = Some(vec!["New starter prompt".to_string()]); + interface.website_url = Some("https://another-host.test".to_string()); + assert_ne!(previous, current); + assert!(installed_plugin_metadata_eq(&previous, ¤t)); + let mut current_catalog = catalog(¤t); + current_catalog[0].display_name = "New marketplace title".to_string(); + assert!(remote_catalog_metadata_eq( + &catalog(&previous), + ¤t_catalog + )); + + current[0].interface = None; + assert!(installed_plugin_metadata_eq(&previous, ¤t)); + assert!(remote_catalog_metadata_eq( + &catalog(&previous), + &catalog(¤t) + )); +} + +#[test] +fn installed_ordering_remains_significant() { + let mut other = plugin("old"); + other.id.push_str("-other"); + other.name.push_str("-other"); + let previous = [plugin("old"), other]; + let mut current = previous.clone(); + current.reverse(); + assert!(!installed_plugin_metadata_eq(&previous, ¤t)); +} + +#[test] +fn store_badge_changes_do_not_invalidate() { + let mut previous = [plugin("old")]; + previous[0].interface.as_mut().unwrap().capabilities = + vec!["tools".to_string(), "skills".to_string()]; + for badges in [vec!["skills", "tools"], vec!["updated badge"], vec![]] { + let mut current = previous.clone(); + current[0].interface.as_mut().unwrap().capabilities = + badges.into_iter().map(str::to_string).collect(); + assert_ne!(previous, current); + assert!(installed_plugin_metadata_eq(&previous, ¤t)); + assert!(remote_catalog_metadata_eq( + &catalog(&previous), + &catalog(¤t) + )); + } +} + +#[test] +fn catalog_display_reordering_does_not_invalidate() { + let mut first = plugin("old"); + first.interface.as_mut().unwrap().display_name = Some("Alpha".to_string()); + let mut second = plugin("old"); + second.id.push_str("-second"); + second.name.push_str("-second"); + second.interface.as_mut().unwrap().display_name = Some("Beta".to_string()); + let previous = [first, second]; + let mut current = previous.clone(); + current[0].interface.as_mut().unwrap().display_name = Some("Zulu".to_string()); + let previous_catalog = catalog(&previous); + let current_catalog = catalog(¤t); + assert_ne!( + previous_catalog[0].plugins[0].id, + current_catalog[0].plugins[0].id + ); + assert!(installed_plugin_metadata_eq(&previous, ¤t)); + assert!(remote_catalog_metadata_eq( + &previous_catalog, + ¤t_catalog + )); + + current[0].version = Some("2.0.0".to_string()); + assert!(!remote_catalog_metadata_eq( + &previous_catalog, + &catalog(¤t) + )); + assert!(!remote_catalog_metadata_eq( + &previous_catalog, + &catalog(¤t[1..]) + )); + current[0] = current[1].clone(); + assert!(!remote_catalog_metadata_eq( + &previous_catalog, + &catalog(¤t) + )); +}