From d905376628cb0cdf66beaac57063dd09650b2b6e Mon Sep 17 00:00:00 2001 From: jif-oai Date: Mon, 13 Apr 2026 20:08:43 +0100 Subject: [PATCH] =?UTF-8?q?feat:=20Avoid=20reloading=20curated=20marketpla?= =?UTF-8?q?ces=20for=20tool-suggest=20discovera=E2=80=A6=20(#17638)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - stop `list_tool_suggest_discoverable_plugins()` from reloading the curated marketplace for each discoverable plugin - reuse a direct plugin-detail loader against the already-resolved marketplace entry The trigger was to stop those logs spamming: ``` d=019d81cf-6f69-7230-98aa-74294ff2dc5a}:submission_dispatch{otel.name="op.dispatch.user_input" submission.id="019d86c8-0a8e-7013-b442-109aabbf75c9" codex.op="user_input"}:turn{otel.name="session_task.turn" thread.id=019d81cf-6f69-7230-98aa-74294ff2dc5a turn.id=019d86c8-0a8e-7013-b442-109aabbf75c9 model=gpt-5.4}: ignoring interface.defaultPrompt: prompt must be at most 128 characters path=/Users/jif/.codex/.tmp/plugins/plugins/life-science-research/.codex-plugin/plugin.json 2026-04-13T12:27:30.402Z WARN [019d81cf-6f69-7230-98aa-74294ff2dc5a] codex_core::plugins::manifest - session_loop{thread_id=019d81cf-6f69-7230-98aa-74294ff2dc5a}:submission_dispatch{otel.name="op.dispatch.user_input" submission.id="019d86c8-0a8e-7013-b442-109aabbf75c9" codex.op="user_input"}:turn{otel.name="session_task.turn" thread.id=019d81cf-6f69-7230-98aa-74294ff2dc5a turn.id=019d86c8-0a8e-7013-b442-109aabbf75c9 model=gpt-5.4}: ignoring interface.defaultPrompt: prompt must be at most 128 characters path=/Users/jif/.codex/.tmp/plugins/plugins/build-ios-apps/.codex-plugin/plugin.json 2026-04-13T12:27:30.402Z WARN [019d81cf-6f69-7230-98aa-74294ff2dc5a] codex_core::plugins::manifest - session_loop{thread_id=019d81cf-6f69-7230-98aa-74294ff2dc5a}:submission_dispatch{otel.name="op.dispatch.user_input" submission.id="019d86c8-0a8e-7013-b442-109aabbf75c9" codex.op="user_input"}:turn{otel.name="session_task.turn" thread.id=019d81cf-6f69-7230-98aa-74294ff2dc5a turn.id=019d86c8-0a8e-7013-b442-109aabbf75c9 model=gpt-5.4}: ignoring interface.defaultPrompt: prompt must be at most 128 characters path=/Users/jif/.codex/.tmp/plugins/plugins/life-science-research/.codex-plugin/plugin.json 2026-04-13T12:27:30.405Z WARN [019d81cf-6f69-7230-98aa-74294ff2dc5a] codex_core::plugins::manifest - session_loop{thread_id=019d81cf-6f69-7230-98aa-74294ff2dc5a}:submission_dispatch{otel.name="op.dispatch.user_input" submission.id="019d86c8-0a8e-7013-b442-109aabbf75c9" codex.op="user_input"}:turn{otel.name="session_task.turn" thread.id=019d81cf-6f69-7230-98aa-74294ff2dc5a turn.id=019d86c8-0a8e-7013-b442-109aabbf75c9 model=gpt-5.4}: ignoring interface.defaultPrompt: prompt must be at most 128 characters path=/Users/jif/.codex/.tmp/plugins/plugins/build-ios-apps/.codex-plugin/plugin.json 2026-04-13T12:27:30.406Z WARN [019d81cf-6f69-7230-98aa-74294ff2dc5a] codex_core::plugins::manifest - session_loop{thread_id=019d81cf-6f69-7230-98aa-74294ff2dc5a}:submission_dispatch{otel.name="op.dispatch.user_input" submission.id="019d86c8-0a8e-7013-b442-109aabbf75c9" codex.op="user_input"}:turn{otel.name="session_task.turn" thread.id=019d81cf-6f69-7230-98aa-74294ff2dc5a turn.id=019d86c8-0a8e-7013-b442-109aabbf75c9 model=gpt-5.4}: ignoring interface.defaultPrompt: prompt must be at most 128 characters path=/Users/jif/.codex/.tmp/plugins/plugins/life-science-research/.codex-plugin/plugin.json 2026-04-13T12:27:30.408Z WARN [019d81cf-6f69-7230-98aa-74294ff2dc5a] codex_core::plugins::manifest - session_loop{thread_id=019d81cf-6f69-7230-98aa-74294ff2dc5a}:submission_dispatch{otel.name="op.dispatch.user_input" submission.id="019d86c8-0a8e-7013-b442-109aabbf75c9" codex.op="user_input"}:turn{otel.name="session_task.turn" thread.id=019d81cf-6f69-7230-98aa-74294ff2dc5a turn.id=019d86c8-0a8e-7013-b442-109aabbf75c9 model=gpt-5.4}: ignoring interface.defaultPrompt: prompt must be at most 128 characters path=/Users/jif/.codex/.tmp/plugins/plugins/build-ios-apps/.codex-plugin/plugin.json ``` --- codex-rs/core/src/plugins/discoverable.rs | 13 ++- .../core/src/plugins/discoverable_tests.rs | 60 +++++++++++++ codex-rs/core/src/plugins/manager.rs | 86 +++++++++++++------ 3 files changed, 123 insertions(+), 36 deletions(-) diff --git a/codex-rs/core/src/plugins/discoverable.rs b/codex-rs/core/src/plugins/discoverable.rs index b1bc8ae9b..91d564bec 100644 --- a/codex-rs/core/src/plugins/discoverable.rs +++ b/codex-rs/core/src/plugins/discoverable.rs @@ -4,7 +4,6 @@ use tracing::warn; use super::OPENAI_CURATED_MARKETPLACE_NAME; use super::PluginCapabilitySummary; -use super::PluginReadRequest; use super::PluginsManager; use crate::config::Config; use codex_config::types::ToolSuggestDiscoverableType; @@ -47,6 +46,7 @@ pub(crate) fn list_tool_suggest_discoverable_plugins( else { return Ok(Vec::new()); }; + let curated_marketplace_name = curated_marketplace.name; let mut discoverable_plugins = Vec::::new(); for plugin in curated_marketplace.plugins { @@ -58,17 +58,14 @@ pub(crate) fn list_tool_suggest_discoverable_plugins( } let plugin_id = plugin.id.clone(); - let plugin_name = plugin.name.clone(); - match plugins_manager.read_plugin_for_config( + match plugins_manager.read_plugin_detail_for_marketplace_plugin( config, - &PluginReadRequest { - plugin_name, - marketplace_path: curated_marketplace.path.clone(), - }, + &curated_marketplace_name, + plugin, ) { Ok(plugin) => { - let plugin: PluginCapabilitySummary = plugin.plugin.into(); + let plugin: PluginCapabilitySummary = plugin.into(); discoverable_plugins.push(DiscoverablePluginInfo { id: plugin.config_name, name: plugin.display_name, diff --git a/codex-rs/core/src/plugins/discoverable_tests.rs b/codex-rs/core/src/plugins/discoverable_tests.rs index 70ac887cb..f17c897fe 100644 --- a/codex-rs/core/src/plugins/discoverable_tests.rs +++ b/codex-rs/core/src/plugins/discoverable_tests.rs @@ -9,6 +9,9 @@ use codex_tools::DiscoverablePluginInfo; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use tempfile::tempdir; +use tracing::Level; +use tracing_subscriber::fmt::format::FmtSpan; +use tracing_test::internal::MockWriter; #[tokio::test] async fn list_tool_suggest_discoverable_plugins_returns_uninstalled_curated_plugins() { @@ -140,3 +143,60 @@ discoverables = [{ type = "plugin", id = "sample@openai-curated" }] }] ); } + +#[tokio::test] +async fn list_tool_suggest_discoverable_plugins_does_not_reload_marketplace_per_plugin() { + let codex_home = tempdir().expect("tempdir should succeed"); + let curated_root = crate::plugins::curated_plugins_repo_path(codex_home.path()); + write_openai_curated_marketplace( + &curated_root, + &["slack", "build-ios-apps", "life-science-research"], + ); + write_plugins_feature_config(codex_home.path()); + + let too_long_prompt = "x".repeat(129); + for plugin_name in ["build-ios-apps", "life-science-research"] { + write_file( + &curated_root.join(format!("plugins/{plugin_name}/.codex-plugin/plugin.json")), + &format!( + r#"{{ + "name": "{plugin_name}", + "description": "Plugin that includes skills, MCP servers, and app connectors", + "interface": {{ + "defaultPrompt": "{too_long_prompt}" + }} +}}"# + ), + ); + } + + let config = load_plugins_config(codex_home.path()).await; + let buffer: &'static std::sync::Mutex> = + Box::leak(Box::new(std::sync::Mutex::new(Vec::new()))); + let subscriber = tracing_subscriber::fmt() + .with_level(true) + .with_ansi(false) + .with_max_level(Level::WARN) + .with_span_events(FmtSpan::NONE) + .with_writer(MockWriter::new(buffer)) + .finish(); + let _guard = tracing::subscriber::set_default(subscriber); + + let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config).unwrap(); + + assert_eq!(discoverable_plugins.len(), 1); + assert_eq!(discoverable_plugins[0].id, "slack@openai-curated"); + + let logs = String::from_utf8(buffer.lock().expect("buffer lock").clone()).expect("utf8 logs"); + assert_eq!(logs.matches("ignoring interface.defaultPrompt").count(), 2); + assert_eq!( + logs.matches("build-ios-apps/.codex-plugin/plugin.json") + .count(), + 1 + ); + assert_eq!( + logs.matches("life-science-research/.codex-plugin/plugin.json") + .count(), + 1 + ); +} diff --git a/codex-rs/core/src/plugins/manager.rs b/codex-rs/core/src/plugins/manager.rs index 17eb26474..bfae2edf2 100644 --- a/codex-rs/core/src/plugins/manager.rs +++ b/codex-rs/core/src/plugins/manager.rs @@ -952,13 +952,6 @@ impl PluginsManager { marketplace_name, }); }; - if !self.restriction_product_matches(plugin.policy.products.as_deref()) { - return Err(MarketplaceError::PluginNotFound { - plugin_name: request.plugin_name.clone(), - marketplace_name, - }); - } - let plugin_id = PluginId::new(plugin.name.clone(), marketplace.name.clone()).map_err( |err| match err { PluginIdError::Invalid(message) => MarketplaceError::InvalidPlugin(message), @@ -966,6 +959,51 @@ impl PluginsManager { )?; let plugin_key = plugin_id.as_key(); let (installed_plugins, enabled_plugins) = self.configured_plugin_states(config); + let plugin = self.read_plugin_detail_for_marketplace_plugin( + config, + &marketplace.name, + ConfiguredMarketplacePlugin { + id: plugin_key.clone(), + name: plugin.name, + source: plugin.source, + policy: plugin.policy, + interface: plugin.interface, + installed: installed_plugins.contains(&plugin_key), + enabled: enabled_plugins.contains(&plugin_key), + }, + )?; + + Ok(PluginReadOutcome { + marketplace_name: if marketplace.name == OPENAI_CURATED_MARKETPLACE_NAME { + OPENAI_CURATED_MARKETPLACE_DISPLAY_NAME.to_string() + } else { + marketplace.name + }, + marketplace_path: marketplace.path, + plugin, + }) + } + + pub(crate) fn read_plugin_detail_for_marketplace_plugin( + &self, + config: &Config, + marketplace_name: &str, + plugin: ConfiguredMarketplacePlugin, + ) -> Result { + if !self.restriction_product_matches(plugin.policy.products.as_deref()) { + return Err(MarketplaceError::PluginNotFound { + plugin_name: plugin.name, + marketplace_name: marketplace_name.to_string(), + }); + } + + let plugin_id = + PluginId::new(plugin.name.clone(), marketplace_name.to_string()).map_err(|err| { + match err { + PluginIdError::Invalid(message) => MarketplaceError::InvalidPlugin(message), + } + })?; + let plugin_key = plugin_id.as_key(); let source_path = match &plugin.source { MarketplacePluginSource::Local { path } => path.clone(), }; @@ -1001,27 +1039,19 @@ impl PluginsManager { mcp_server_names.sort_unstable(); mcp_server_names.dedup(); - Ok(PluginReadOutcome { - marketplace_name: if marketplace.name == OPENAI_CURATED_MARKETPLACE_NAME { - OPENAI_CURATED_MARKETPLACE_DISPLAY_NAME.to_string() - } else { - marketplace.name - }, - marketplace_path: marketplace.path, - plugin: PluginDetail { - id: plugin_key.clone(), - name: plugin.name, - description, - source: plugin.source, - policy: plugin.policy, - interface: plugin.interface, - installed: installed_plugins.contains(&plugin_key), - enabled: enabled_plugins.contains(&plugin_key), - skills: resolved_skills.skills, - disabled_skill_paths: resolved_skills.disabled_skill_paths, - apps, - mcp_server_names, - }, + Ok(PluginDetail { + id: plugin_key, + name: plugin.name, + description, + source: plugin.source, + policy: plugin.policy, + interface: plugin.interface, + installed: plugin.installed, + enabled: plugin.enabled, + skills: resolved_skills.skills, + disabled_skill_paths: resolved_skills.disabled_skill_paths, + apps, + mcp_server_names, }) }