Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 53 additions & 14 deletions codex-rs/core-plugins/src/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@ use crate::manifest::PluginManifestMcpServers;
use crate::manifest::PluginManifestPaths;
use crate::manifest::load_plugin_manifest;
use crate::marketplace::MarketplacePluginSource;
use crate::marketplace::find_marketplace_plugin;
use crate::marketplace::list_marketplaces;
use crate::marketplace::load_marketplace;
use crate::remote::REMOTE_GLOBAL_MARKETPLACE_NAME;
use crate::remote::RemoteInstalledPlugin;
use crate::store::PluginStore;
use crate::store::plugin_version_for_source;
use crate::store::plugin_version_for_source_with_fallback_manifest;
use codex_app_server_protocol::AuthMode;
use codex_config::ConfigLayerStack;
use codex_config::HooksFile;
Expand Down Expand Up @@ -460,7 +462,7 @@ fn refresh_non_curated_plugin_cache_with_mode(
let store = PluginStore::try_new(codex_home.to_path_buf()).map_err(|err| err.to_string())?;
let marketplace_outcome = list_marketplaces(additional_roots)
.map_err(|err| format!("failed to discover marketplaces for cache refresh: {err}"))?;
let mut plugin_sources = HashMap::<String, MarketplacePluginSource>::new();
let mut plugin_sources = HashMap::<String, (MarketplacePluginSource, Option<String>)>::new();

for marketplace in marketplace_outcome.marketplaces {
if is_openai_curated_marketplace_name(&marketplace.name) {
Expand Down Expand Up @@ -489,14 +491,31 @@ fn refresh_non_curated_plugin_cache_with_mode(
continue;
}

plugin_sources.insert(plugin_key, plugin.source);
let manifest_fallback = find_marketplace_plugin(&marketplace.path, &plugin.name)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-curated cache refresh now retains fallback JSON with each source. That matters because refresh runs later than marketplace discovery and otherwise loses the metadata needed to compute the fallback version and reinstall a manifest-less plugin.

.map(|resolved| {
resolved
.manifest_fallback
.contents_if_has_metadata()
.map(str::to_string)
})
.unwrap_or_else(|err| {
warn!(
plugin = plugin.name,
marketplace = marketplace.name,
error = %err,
"failed to resolve marketplace plugin manifest fallback during cache refresh"
);
None
});
plugin_sources.insert(plugin_key, (plugin.source, manifest_fallback));
}
}

let mut cache_refreshed = false;
for plugin_id in configured_non_curated_plugin_ids {
let plugin_key = plugin_id.as_key();
let Some(source) = plugin_sources.get(&plugin_key).cloned() else {
let Some((source, manifest_fallback_contents)) = plugin_sources.get(&plugin_key).cloned()
else {
warn!(
plugin = plugin_id.plugin_name,
marketplace = plugin_id.marketplace_name,
Expand All @@ -509,18 +528,31 @@ fn refresh_non_curated_plugin_cache_with_mode(
format!("failed to materialize plugin source for {plugin_key}: {err}")
})?;
let source_path = materialized.path.clone();
let plugin_version = plugin_version_for_source(source_path.as_path())
.map_err(|err| format!("failed to read plugin version for {plugin_key}: {err}"))?;
let plugin_version = match manifest_fallback_contents.as_deref() {
Some(manifest_contents) => plugin_version_for_source_with_fallback_manifest(
source_path.as_path(),
manifest_contents,
),
None => plugin_version_for_source(source_path.as_path()),
}
.map_err(|err| format!("failed to read plugin version for {plugin_key}: {err}"))?;

if mode == NonCuratedCacheRefreshMode::IfVersionChanged
&& store.active_plugin_version(&plugin_id).as_deref() == Some(plugin_version.as_str())
{
continue;
}

store
.install_with_version(source_path, plugin_id.clone(), plugin_version)
.map_err(|err| format!("failed to refresh plugin cache for {plugin_key}: {err}"))?;
match manifest_fallback_contents.as_deref() {
Some(manifest_contents) => store.install_with_version_and_fallback_manifest(
source_path,
plugin_id.clone(),
plugin_version,
manifest_contents,
),
None => store.install_with_version(source_path, plugin_id.clone(), plugin_version),
}
.map_err(|err| format!("failed to refresh plugin cache for {plugin_key}: {err}"))?;
cache_refreshed = true;
}

Expand Down Expand Up @@ -901,15 +933,22 @@ fn default_mcp_config_paths(plugin_root: &Path) -> Vec<AbsolutePathBuf> {

pub async fn load_plugin_apps(plugin_root: &Path) -> Vec<AppDeclaration> {
if let Some(manifest) = load_plugin_manifest(plugin_root) {
return load_apps_from_paths(
plugin_root,
plugin_app_config_paths(plugin_root, &manifest.paths),
)
.await;
return load_plugin_apps_from_manifest(plugin_root, &manifest.paths).await;
}
load_apps_from_paths(plugin_root, default_app_config_paths(plugin_root)).await
}

pub(crate) async fn load_plugin_apps_from_manifest(
plugin_root: &Path,
manifest_paths: &PluginManifestPaths,
) -> Vec<AppDeclaration> {
load_apps_from_paths(
plugin_root,
plugin_app_config_paths(plugin_root, manifest_paths),
)
.await
}

pub fn plugin_app_declarations_from_value(value: &JsonValue) -> Vec<AppDeclaration> {
let Ok(parsed) = serde_json::from_value::<PluginAppFile>(value.clone()) else {
return Vec::new();
Expand Down Expand Up @@ -1181,7 +1220,7 @@ async fn load_declared_plugin_mcp_servers(plugin_root: &Path) -> HashMap<String,
.await
}

async fn load_plugin_mcp_servers_from_manifest(
pub(crate) async fn load_plugin_mcp_servers_from_manifest(
plugin_root: &Path,
manifest_paths: &PluginManifestPaths,
plugin_policy: Option<&HashMap<String, PluginMcpServerConfig>>,
Expand Down
63 changes: 52 additions & 11 deletions codex-rs/core-plugins/src/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ 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_apps;
use crate::loader::load_plugin_apps_from_manifest;
use crate::loader::load_plugin_hooks;
use crate::loader::load_plugin_hooks_from_layer_stack;
use crate::loader::load_plugin_mcp_servers;
use crate::loader::load_plugin_mcp_servers_from_manifest;
use crate::loader::load_plugin_skills;
use crate::loader::load_plugins_from_layer_stack;
use crate::loader::log_plugin_load_errors;
Expand All @@ -27,6 +27,7 @@ use crate::marketplace::MarketplaceInterface;
use crate::marketplace::MarketplaceListError;
use crate::marketplace::MarketplaceListOutcome;
use crate::marketplace::MarketplacePluginAuthPolicy;
use crate::marketplace::MarketplacePluginManifestFallback;
use crate::marketplace::MarketplacePluginPolicy;
use crate::marketplace::MarketplacePluginSource;
use crate::marketplace::ResolvedMarketplacePlugin;
Expand Down Expand Up @@ -315,6 +316,7 @@ pub struct ConfiguredMarketplacePlugin {
pub policy: MarketplacePluginPolicy,
pub interface: Option<PluginManifestInterface>,
pub keywords: Vec<String>,
pub manifest_fallback: Option<MarketplacePluginManifestFallback>,
pub installed: bool,
pub enabled: bool,
}
Expand Down Expand Up @@ -1256,15 +1258,32 @@ impl PluginsManager {
};
let store = self.store.clone();
let codex_home = self.codex_home.clone();
let manifest_fallback_contents = resolved

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Install passes fallback contents directly into PluginStore. There is no prepare-source copy here anymore; Store owns validation and injection inside its existing atomic cache copy, while the four match arms preserve explicit-version and inferred-version behavior.

.manifest_fallback
.contents_if_has_metadata()
.map(str::to_string);
let result: StorePluginInstallResult = tokio::task::spawn_blocking(move || {
let materialized =
materialize_marketplace_plugin_source(codex_home.as_path(), &resolved.source)
.map_err(PluginStoreError::Invalid)?;
let source_path = materialized.path;
if let Some(plugin_version) = plugin_version {
store.install_with_version(source_path, resolved.plugin_id, plugin_version)
} else {
store.install(source_path, resolved.plugin_id)
match (plugin_version, manifest_fallback_contents.as_deref()) {
(Some(plugin_version), Some(manifest_contents)) => store
.install_with_version_and_fallback_manifest(
source_path,
resolved.plugin_id,
plugin_version,
manifest_contents,
),
(Some(plugin_version), None) => {
store.install_with_version(source_path, resolved.plugin_id, plugin_version)
}
(None, Some(manifest_contents)) => store.install_with_fallback_manifest(
source_path,
resolved.plugin_id,
manifest_contents,
),
(None, None) => store.install(source_path, resolved.plugin_id),
}
})
.await
Expand Down Expand Up @@ -1394,6 +1413,7 @@ impl PluginsManager {
let enabled = enabled_plugins.contains(&plugin_key);
let mut interface = plugin.interface;
let mut local_version = plugin.local_version;
let manifest_fallback = plugin.manifest_fallback.clone();
if installed
&& matches!(&plugin.source, MarketplacePluginSource::Git { .. })
&& let Some(plugin_id) = plugin_id.as_ref()
Expand Down Expand Up @@ -1424,6 +1444,7 @@ impl PluginsManager {
policy: plugin.policy,
keywords: plugin.keywords,
interface,
manifest_fallback,
})
})
.collect::<Vec<_>>();
Expand Down Expand Up @@ -1491,6 +1512,10 @@ impl PluginsManager {

let marketplace_name = plugin.plugin_id.marketplace_name.clone();
let plugin_key = plugin.plugin_id.as_key();
let manifest_fallback = plugin
.manifest_fallback
.contents_if_has_metadata()
.map(|_| plugin.manifest_fallback.clone());
let (installed_plugins, enabled_plugins) = self.configured_plugin_states(config);
let installed = installed_plugins.contains(&plugin_key);
let installed_version = if installed {
Expand Down Expand Up @@ -1518,6 +1543,7 @@ impl PluginsManager {
.as_ref()
.map(|manifest| manifest.keywords.clone())
.unwrap_or_default(),
manifest_fallback,
installed,
enabled: enabled_plugins.contains(&plugin_key),
},
Expand Down Expand Up @@ -1604,9 +1630,18 @@ impl PluginsManager {
"path does not exist or is not a directory".to_string(),
));
}
let manifest = load_plugin_manifest(source_path.as_path()).ok_or_else(|| {
MarketplaceError::InvalidPlugin("missing or invalid plugin.json".to_string())
})?;
let manifest =
if codex_utils_plugins::find_plugin_manifest_path(source_path.as_path()).is_some() {
load_plugin_manifest(source_path.as_path())
} else {
plugin
.manifest_fallback
.as_ref()
.and_then(|fallback| fallback.parse_for_plugin_root(source_path.as_path()))
}
.ok_or_else(|| {
MarketplaceError::InvalidPlugin("missing or invalid plugin.json".to_string())
})?;
let description = manifest.description.clone();
let marketplace_category = plugin
.interface
Expand Down Expand Up @@ -1637,8 +1672,14 @@ impl PluginsManager {
})
.collect();
let auth_mode = self.auth_mode();
let mut app_declarations = load_plugin_apps(source_path.as_path()).await;
let mut mcp_servers = load_plugin_mcp_servers(source_path.as_path(), auth_mode).await;
let mut app_declarations =

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These loaders now consume the paths from the manifest that was already selected above. This fixes the previous gap where detail reading accepted a fallback manifest, then app and MCP loading reread disk and dropped fallback-declared paths.

load_plugin_apps_from_manifest(source_path.as_path(), &manifest.paths).await;
let mut mcp_servers = load_plugin_mcp_servers_from_manifest(
source_path.as_path(),
&manifest.paths,
/*plugin_policy*/ None,
)
.await;
if auth_mode.is_some() {
apply_app_mcp_routing_policy(
&mut app_declarations,
Expand Down
Loading
Loading