openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-11-10 21:00:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

Trace MCP refresh causes and plugin metadata changes (#60505)

Browse Source 
## What changed

- Track and log `McpRefreshReason` values for runtime refreshes, including invalidations received during thread startup and forced refreshes. Restore claimed reasons when a refresh is cancelled before publication, while preserving concurrent invalidations.
- Correlate installed-plugin metadata invalidations with downstream MCP refreshes using a shared change ID. Log changed field names by plugin ID, additions, removals, and reordering without logging field values.
- Record whether background installed-plugin bundle updates were requested by startup, plugin listing, or installation.

## Testing

Add coverage for metadata change attribution across membership changes and reordering, and for retaining refresh causes across cancellation and concurrent invalidation.

GitOrigin-RevId: 3d4485ccfcc1d8788eb9909a060ac200c467af13
This commit is contained in:
Charlie Marsh
2026-11-10 02:45:30 +00:00
committed by copyberry
parent 0e6f153732
commit aa39ded153
28 changed files with 534 additions and 196 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core-plugins/src/lib.rs
+1
View File
@@ -86,6 +86,7 @@ pub use manager::ConfiguredMarketplace;
pub use manager::ConfiguredMarketplaceListOutcome;
pub use manager::ConfiguredMarketplacePlugin;
pub use manager::EffectivePluginsChange;
pub use manager::InstalledPluginSyncSource;
pub use manager::PluginDetail;
pub use manager::PluginDetailsUnavailableReason;
pub use manager::PluginInstallError;
codex-rs/core-plugins/src/manager.rs
+71 -28
View File
@@ -254,9 +254,19 @@ impl PluginsConfigInput {
}
}
/// Operation that requested a background sync of installed plugin bundles.
#[derive(Clone, Copy, Debug)]
pub enum InstalledPluginSyncSource {
PluginInstalled,
PluginList,
Startup,
}
/// Effective-plugin changes that downstream composition layers may act on.
#[derive(Debug, Clone, Default, PartialEq, Eq)]
pub struct EffectivePluginsChange {
/// Joins this notification to its installed-plugin cache publication in diagnostics.
pub metadata_change_id: Option<uuid::Uuid>,
/// Remote bundles installed or updated by background installed-plugin sync.
pub materialized_remote_plugins: Vec<RemotePluginMaterialization>,
}
@@ -320,6 +330,10 @@ struct RemoteInstalledPluginsCache {
display_assets: InstalledPluginDisplayAssets,
}
struct InstalledPluginCacheUpdate {
metadata_change_id: Option<uuid::Uuid>,
}
enum RemoteInstalledPluginsCachePublication {
Refresh(InstalledPluginDisplayAssets),
Reconcile,
@@ -1435,7 +1449,7 @@ impl PluginsManager {
&plugins,
visible_marketplaces,
);
let Some(changed) = self.write_remote_installed_plugins_cache_snapshot(
let Some(update) = self.write_remote_installed_plugins_cache_snapshot(
generation,
plugins,
auth,
@@ -1453,8 +1467,13 @@ impl PluginsManager {
)
.unwrap_or(marketplaces));
};
if changed && let Some(on_effective_plugins_changed) = on_effective_plugins_changed {
on_effective_plugins_changed(EffectivePluginsChange::default());
if update.metadata_change_id.is_some()
&& let Some(on_effective_plugins_changed) = on_effective_plugins_changed
{
on_effective_plugins_changed(EffectivePluginsChange {
metadata_change_id: update.metadata_change_id,
..Default::default()
});
}
Ok(self
.build_remote_installed_plugin_marketplaces_from_cache_for_auth(
@@ -1545,7 +1564,7 @@ impl PluginsManager {
plugins: Vec<RemoteInstalledPlugin>,
auth: Option<&CodexAuth>,
publication: RemoteInstalledPluginsCachePublication,
) -> Option<bool> {
) -> Option<InstalledPluginCacheUpdate> {
let auth_identity = RemoteInstalledPluginsAuthIdentity::from_auth(auth);
if !self.remote_installed_plugins_auth_is_current(&auth_identity) {
return None;
@@ -1584,23 +1603,32 @@ impl PluginsManager {
InstalledPluginDisplayAssets::Include
}
};
if cache.plugins.as_ref() == Some(&plugins) {
drop(cache);
if needs_effective_plugins_refresh {
self.clear_loaded_plugins_cache();
}
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);
let previous = cache.plugins.as_deref();
let payload_changed = previous != Some(plugins.as_slice());
let metadata_changed = payload_changed
&& previous.is_none_or(|old| {
!crate::remote_metadata::installed_plugin_metadata_eq(old, &plugins)
});
let changed = needs_effective_plugins_refresh || metadata_changed;
if changed {
let metadata_change_id = changed.then(uuid::Uuid::new_v4);
let initial_snapshot = previous.is_none();
let changes = if metadata_changed && tracing::enabled!(tracing::Level::INFO) {
crate::remote_metadata::installed_plugin_metadata_changes(
previous.unwrap_or_default(),
&plugins,
)
} else {
Default::default()
};
if payload_changed {
cache.plugins = Some(plugins);
}
drop(cache);
if let Some(change_id) = metadata_change_id {
tracing::info!(%change_id, ?changes, initial_snapshot, reconciliation_retry = needs_effective_plugins_refresh, is_reconcile, "installed plugin runtime metadata invalidated");
self.clear_loaded_plugins_cache();
}
Some(changed)
Some(InstalledPluginCacheUpdate { metadata_change_id })
}
#[cfg(test)]
@@ -1616,6 +1644,8 @@ impl PluginsManager {
RemoteInstalledPluginsCachePublication::Refresh(InstalledPluginDisplayAssets::Include),
)
.expect("test cache generation should remain current")
.metadata_change_id
.is_some()
}
pub fn clear_remote_installed_plugins_cache(&self) -> bool {
@@ -1727,6 +1757,7 @@ impl PluginsManager {
config: &PluginsConfigInput,
auth: Option<CodexAuth>,
on_effective_plugins_changed: Option<EffectivePluginsChangedCallback>,
source: InstalledPluginSyncSource,
) {
if !config.plugins_enabled {
return;
@@ -1748,18 +1779,21 @@ impl PluginsManager {
.reconcile_remote_installed_plugins_after_acquiring_gate(&config, Some(&auth))
.await;
match result {
Ok((outcome, effective_plugins_changed)) => {
Ok((outcome, update)) => {
tracing::info!(
materialized_remote_plugins = ?outcome.materialized_remote_plugins,
changed_plugins = ?outcome.changed_plugins,
failed_remote_plugin_ids = ?outcome.failed_remote_plugin_ids,
failed_materialization_remote_plugin_ids = ?outcome.failed_materialization_remote_plugin_ids,
?source,
metadata_change_id = ?update.metadata_change_id,
"completed remote installed plugin bundle sync"
);
if (effective_plugins_changed || !outcome.changed_plugins.is_empty())
if (update.metadata_change_id.is_some() || !outcome.changed_plugins.is_empty())
&& let Some(on_effective_plugins_changed) = on_effective_plugins_changed
{
on_effective_plugins_changed(EffectivePluginsChange {
metadata_change_id: update.metadata_change_id,
materialized_remote_plugins: outcome.materialized_remote_plugins,
});
}
@@ -1817,8 +1851,13 @@ impl PluginsManager {
&self,
config: &PluginsConfigInput,
auth: Option<&CodexAuth>,
) -> Result<(RemoteInstalledPluginBundleSyncOutcome, bool), RemoteInstalledPluginBundleSyncError>
{
) -> Result<
(
RemoteInstalledPluginBundleSyncOutcome,
InstalledPluginCacheUpdate,
),
RemoteInstalledPluginBundleSyncError,
> {
let generation = self
.begin_remote_installed_plugins_reconcile(auth)
.ok_or(RemoteInstalledPluginBundleSyncError::Superseded)?;
@@ -1893,7 +1932,7 @@ impl PluginsManager {
outcome
.changed_plugins
.sort_unstable_by(|left, right| left.plugin_id.cmp(&right.plugin_id));
let Some(effective_plugins_changed) = self.write_remote_installed_plugins_cache_snapshot(
let Some(update) = self.write_remote_installed_plugins_cache_snapshot(
generation,
result.installed_plugins,
auth,
@@ -1902,10 +1941,10 @@ impl PluginsManager {
return Err(RemoteInstalledPluginBundleSyncError::Superseded);
};
reconciliation.committed = true;
if !effective_plugins_changed && !outcome.changed_plugins.is_empty() {
if update.metadata_change_id.is_none() && !outcome.changed_plugins.is_empty() {
self.clear_loaded_plugins_cache();
}
Ok((outcome, effective_plugins_changed))
Ok((outcome, update))
}
fn maybe_start_remote_catalog_cache_refresh(
@@ -1947,6 +1986,7 @@ impl PluginsManager {
&context.global_config,
auth,
on_effective_plugins_changed,
InstalledPluginSyncSource::PluginList,
);
}
@@ -3041,6 +3081,7 @@ impl PluginsManager {
&config_for_remote_sync,
auth,
on_effective_plugins_changed,
InstalledPluginSyncSource::Startup,
);
});
}
@@ -3467,7 +3508,7 @@ impl PluginsManager {
.await;
match plugins {
Ok(plugins) => {
let Some(changed) = self.write_remote_installed_plugins_cache_snapshot(
let Some(update) = self.write_remote_installed_plugins_cache_snapshot(
request.generation,
plugins,
request.auth.as_ref(),
@@ -3477,7 +3518,7 @@ impl PluginsManager {
) else {
continue;
};
let should_notify = changed
let should_notify = update.metadata_change_id.is_some()
|| !request.change.materialized_remote_plugins.is_empty()
|| matches!(
request.notify,
@@ -3487,7 +3528,9 @@ impl PluginsManager {
&& let Some(on_effective_plugins_changed) =
request.on_effective_plugins_changed
{
on_effective_plugins_changed(request.change);
let mut change = request.change;
change.metadata_change_id = update.metadata_change_id;
on_effective_plugins_changed(change);
}
}
Err(
codex-rs/core-plugins/src/manager_tests.rs
+67 -42
View File
@@ -1860,12 +1860,14 @@ async fn unresolved_display_assets_does_not_satisfy_display_cache_reads() {
.prepare_remote_installed_plugins_cache_generation(auth.as_ref())
.unwrap();
assert_eq!(
manager.write_remote_installed_plugins_cache_snapshot(
generation,
plugins.clone(),
auth.as_ref(),
RemoteInstalledPluginsCachePublication::Refresh(InstalledPluginDisplayAssets::Skip),
),
manager
.write_remote_installed_plugins_cache_snapshot(
generation,
plugins.clone(),
auth.as_ref(),
RemoteInstalledPluginsCachePublication::Refresh(InstalledPluginDisplayAssets::Skip),
)
.map(|update| update.metadata_change_id.is_some()),
Some(true)
);
assert!(
@@ -1883,12 +1885,16 @@ async fn unresolved_display_assets_does_not_satisfy_display_cache_reads() {
.expect("runtime metadata is ready");
assert_eq!(runtime[0].plugins.len(), 1);
assert_eq!(
manager.write_remote_installed_plugins_cache_snapshot(
generation,
plugins.clone(),
auth.as_ref(),
RemoteInstalledPluginsCachePublication::Refresh(InstalledPluginDisplayAssets::Include),
),
manager
.write_remote_installed_plugins_cache_snapshot(
generation,
plugins.clone(),
auth.as_ref(),
RemoteInstalledPluginsCachePublication::Refresh(
InstalledPluginDisplayAssets::Include
),
)
.map(|update| update.metadata_change_id.is_some()),
Some(false)
);
assert_eq!(
@@ -1898,12 +1904,14 @@ async fn unresolved_display_assets_does_not_satisfy_display_cache_reads() {
Some(runtime)
);
assert_eq!(
manager.write_remote_installed_plugins_cache_snapshot(
generation,
plugins,
auth.as_ref(),
RemoteInstalledPluginsCachePublication::Refresh(InstalledPluginDisplayAssets::Skip),
),
manager
.write_remote_installed_plugins_cache_snapshot(
generation,
plugins,
auth.as_ref(),
RemoteInstalledPluginsCachePublication::Refresh(InstalledPluginDisplayAssets::Skip),
)
.map(|update| update.metadata_change_id.is_some()),
Some(false)
);
assert!(
@@ -7454,6 +7462,7 @@ fn remote_installed_plugins_cache_refresh_coalesces_materializations() {
authenticated_account_id: Some("account-123".to_string()),
};
let change = |name: &str| EffectivePluginsChange {
metadata_change_id: None,
materialized_remote_plugins: vec![materialization(name)],
};
let callback = |count: std::sync::Arc<std::sync::atomic::AtomicUsize>| {
@@ -7496,6 +7505,7 @@ fn remote_installed_plugins_cache_refresh_coalesces_materializations() {
assert_eq!(
request.change,
EffectivePluginsChange {
metadata_change_id: None,
materialized_remote_plugins: vec![materialization("alpha"), materialization("beta"),],
}
);
@@ -7557,11 +7567,13 @@ remote_plugin = true
&config,
Some(auth.clone()),
/*on_effective_plugins_changed*/ None,
InstalledPluginSyncSource::PluginInstalled,
);
second_manager.maybe_start_remote_installed_plugin_bundle_sync(
&config,
Some(auth),
/*on_effective_plugins_changed*/ None,
InstalledPluginSyncSource::PluginInstalled,
);
let _guard = first_manager
@@ -7853,6 +7865,7 @@ async fn failed_installed_plugin_pagination_preserves_inventory_and_releases_mut
&config,
Some(auth),
/*on_effective_plugins_changed*/ None,
InstalledPluginSyncSource::PluginInstalled,
);
// Starting the sync acquires the gate synchronously, so this waits for its release.
let _guard = tokio::time::timeout(
@@ -7905,21 +7918,27 @@ fn reconciliation_fences_refresh_publication_and_recovers_after_cancellation() {
.expect("capture concurrent refresh generation");
assert_eq!(
manager.write_remote_installed_plugins_cache_snapshot(
stale_refresh_generation,
Vec::new(),
/*auth*/ None,
RemoteInstalledPluginsCachePublication::Refresh(InstalledPluginDisplayAssets::Include),
),
manager
.write_remote_installed_plugins_cache_snapshot(
stale_refresh_generation,
Vec::new(),
/*auth*/ None,
RemoteInstalledPluginsCachePublication::Refresh(
InstalledPluginDisplayAssets::Include
),
)
.map(|update| update.metadata_change_id.is_some()),
None
);
assert_eq!(
manager.write_remote_installed_plugins_cache_snapshot(
reconcile_generation,
vec![remote_installed_linear_plugin()],
/*auth*/ None,
RemoteInstalledPluginsCachePublication::Reconcile,
),
manager
.write_remote_installed_plugins_cache_snapshot(
reconcile_generation,
vec![remote_installed_linear_plugin()],
/*auth*/ None,
RemoteInstalledPluginsCachePublication::Reconcile,
)
.map(|update| update.metadata_change_id.is_some()),
Some(true)
);
let cancelled_generation = manager
@@ -7937,24 +7956,30 @@ fn reconciliation_fences_refresh_publication_and_recovers_after_cancellation() {
.remote_installed_plugins_cache_generation_if_current(/*auth*/ None)
.expect("capture refresh generation after cancellation");
assert_eq!(
manager.write_remote_installed_plugins_cache_snapshot(
refresh_generation,
vec![remote_installed_plugin("beta")],
/*auth*/ None,
RemoteInstalledPluginsCachePublication::Refresh(InstalledPluginDisplayAssets::Include),
),
manager
.write_remote_installed_plugins_cache_snapshot(
refresh_generation,
vec![remote_installed_plugin("beta")],
/*auth*/ None,
RemoteInstalledPluginsCachePublication::Refresh(
InstalledPluginDisplayAssets::Include
),
)
.map(|update| update.metadata_change_id.is_some()),
Some(true)
);
let retry_generation = manager
.begin_remote_installed_plugins_reconcile(/*auth*/ None)
.expect("begin retry after cancellation");
assert_eq!(
manager.write_remote_installed_plugins_cache_snapshot(
retry_generation,
vec![remote_installed_plugin("beta")],
/*auth*/ None,
RemoteInstalledPluginsCachePublication::Reconcile,
),
manager
.write_remote_installed_plugins_cache_snapshot(
retry_generation,
vec![remote_installed_plugin("beta")],
/*auth*/ None,
RemoteInstalledPluginsCachePublication::Reconcile,
)
.map(|update| update.metadata_change_id.is_some()),
Some(true)
);
}
codex-rs/core-plugins/src/remote_metadata.rs
+71 -17
View File
@@ -5,29 +5,83 @@
use crate::remote::RemoteInstalledPlugin;
use crate::remote::RemoteMarketplace;
use std::collections::BTreeMap;
use std::collections::HashMap;
/// Use the same comparisons for invalidation and its diagnostics. Expose field names only.
fn changed_fields<'a>(
previous: &'a RemoteInstalledPlugin,
current: &'a RemoteInstalledPlugin,
) -> impl Iterator<Item = &'static str> + 'a {
macro_rules! compare {
($($field:ident),+ $(,)?) => {
[$( (stringify!($field), previous.$field != current.$field) ),+]
};
}
compare!(
marketplace_name,
id,
name,
canonical_app_id,
version,
installed_at,
enabled,
install_policy,
install_policy_source,
must_show_installation_interstitial,
auth_policy,
availability,
disabled_reason,
eligible_plan_types,
)
.into_iter()
.filter_map(|(field, changed)| changed.then_some(field))
}
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
})
&& previous
.iter()
.zip(current)
.all(|(previous, current)| changed_fields(previous, current).next().is_none())
}
/// Called only for a changed snapshot. Match by remote ID so an insertion does
/// not misattribute every subsequent plugin's fields to its neighbor.
pub(crate) fn installed_plugin_metadata_changes(
previous: &[RemoteInstalledPlugin],
current: &[RemoteInstalledPlugin],
) -> BTreeMap<String, Vec<&'static str>> {
let mut old: HashMap<_, _> = previous
.iter()
.enumerate()
.map(|(index, p)| (p.id.as_str(), (index, p)))
.collect();
// A removed plugin shifts subsequent indices without reordering the surviving list.
let same_members =
previous.len() == current.len() && current.iter().all(|p| old.contains_key(p.id.as_str()));
let mut changes = BTreeMap::new();
for (index, plugin) in current.iter().enumerate() {
let fields = if let Some((old_index, prior)) = old.remove(plugin.id.as_str()) {
let mut fields: Vec<_> = changed_fields(prior, plugin).collect();
if old_index != index && same_members {
fields.push("position");
}
fields
} else {
vec!["added"]
};
if !fields.is_empty() {
changes.insert(plugin.id.clone(), fields);
}
}
for id in old.into_keys() {
changes.insert(id.to_owned(), vec!["removed"]);
}
changes
}
/// Compare installed catalog identity, versions, enablement, policy and availability.
codex-rs/core-plugins/src/remote_metadata_tests.rs
+37
View File
@@ -121,6 +121,13 @@ fn installed_ordering_remains_significant() {
let mut current = previous.clone();
current.reverse();
assert!(!installed_plugin_metadata_eq(&previous, &current));
assert_eq!(
installed_plugin_metadata_changes(&previous, &current),
BTreeMap::from([
(previous[0].id.clone(), vec!["position"]),
(previous[1].id.clone(), vec!["position"]),
])
);
}
#[test]
@@ -179,3 +186,33 @@ fn catalog_display_reordering_does_not_invalidate() {
&catalog(&current)
));
}
#[test]
fn diagnostics_attribute_fields_by_plugin_id_across_membership_changes() {
let previous = [
plugin("old"),
RemoteInstalledPlugin {
id: "removed".into(),
..plugin("old")
},
];
let mut updated = previous[0].clone();
updated.enabled = !updated.enabled;
updated.version = Some("2".into());
let id = updated.id.clone();
let current = [
RemoteInstalledPlugin {
id: "added".into(),
..plugin("new")
},
updated,
];
assert_eq!(
installed_plugin_metadata_changes(&previous, &current),
std::collections::BTreeMap::from([
("added".into(), vec!["added"]),
("removed".into(), vec!["removed"]),
(id, vec!["version", "enabled"]),
])
);
}