mirror of
https://github.com/openai/codex.git
synced 2026-10-31 19:28:49 +00:00
plugins: harden install reconciliation
This commit is contained in:
@@ -47,7 +47,7 @@ pub async fn reconcile_user_plugin_registrations(
|
||||
plugin_key: String,
|
||||
plugin_name: String,
|
||||
enable_canonical: bool,
|
||||
) -> std::io::Result<()> {
|
||||
) -> std::io::Result<Vec<String>> {
|
||||
let target_path = codex_home.join(CONFIG_TOML_FILE);
|
||||
let mut paths = BTreeSet::new();
|
||||
if let Some(config_layer_stack) = config_layer_stack {
|
||||
@@ -62,10 +62,24 @@ pub async fn reconcile_user_plugin_registrations(
|
||||
}
|
||||
paths.remove(&target_path);
|
||||
task::spawn_blocking(move || {
|
||||
let mut removed_plugin_keys = BTreeSet::new();
|
||||
for path in paths {
|
||||
reconcile_plugin_config_path(&path, &plugin_key, &plugin_name, /*enable*/ false)?;
|
||||
removed_plugin_keys.extend(reconcile_plugin_config_path(
|
||||
&path,
|
||||
&plugin_key,
|
||||
&plugin_name,
|
||||
enable_canonical,
|
||||
/*is_target*/ false,
|
||||
)?);
|
||||
}
|
||||
reconcile_plugin_config_path(&target_path, &plugin_key, &plugin_name, enable_canonical)
|
||||
removed_plugin_keys.extend(reconcile_plugin_config_path(
|
||||
&target_path,
|
||||
&plugin_key,
|
||||
&plugin_name,
|
||||
enable_canonical,
|
||||
/*is_target*/ true,
|
||||
)?);
|
||||
Ok(removed_plugin_keys.into_iter().collect())
|
||||
})
|
||||
.await
|
||||
.map_err(|err| std::io::Error::other(format!("config persistence task panicked: {err}")))?
|
||||
@@ -112,19 +126,24 @@ fn reconcile_plugin_config_path(
|
||||
config_path: &Path,
|
||||
plugin_key: &str,
|
||||
plugin_name: &str,
|
||||
enable: bool,
|
||||
) -> std::io::Result<()> {
|
||||
enable_canonical: bool,
|
||||
is_target: bool,
|
||||
) -> std::io::Result<Vec<String>> {
|
||||
let write_paths = resolve_symlink_write_paths(config_path)?;
|
||||
let mut doc = read_or_create_document(write_paths.read_path.as_deref())?;
|
||||
let mutated = if enable {
|
||||
set_plugin_enabled_exclusively(&mut doc, plugin_key, plugin_name)
|
||||
let removed_plugin_keys = remove_other_plugin_registrations(&mut doc, plugin_key, plugin_name);
|
||||
let canonical_mutated = if enable_canonical && is_target {
|
||||
set_plugin_enabled(&mut doc, plugin_key, /*enabled*/ true)
|
||||
} else if enable_canonical {
|
||||
clear_plugin_enabled(&mut doc, plugin_key)
|
||||
} else {
|
||||
remove_other_plugin_registrations(&mut doc, plugin_key, plugin_name)
|
||||
false
|
||||
};
|
||||
let mutated = canonical_mutated || !removed_plugin_keys.is_empty();
|
||||
if mutated {
|
||||
write_atomically(&write_paths.write_path, &doc.to_string())?;
|
||||
}
|
||||
Ok(())
|
||||
Ok(removed_plugin_keys)
|
||||
}
|
||||
|
||||
fn read_or_create_document(config_path: Option<&Path>) -> std::io::Result<DocumentMut> {
|
||||
@@ -155,26 +174,17 @@ fn set_plugin_enabled(doc: &mut DocumentMut, plugin_key: &str, enabled: bool) ->
|
||||
true
|
||||
}
|
||||
|
||||
fn set_plugin_enabled_exclusively(
|
||||
doc: &mut DocumentMut,
|
||||
plugin_key: &str,
|
||||
plugin_name: &str,
|
||||
) -> bool {
|
||||
let removed_duplicate = remove_other_plugin_registrations(doc, plugin_key, plugin_name);
|
||||
set_plugin_enabled(doc, plugin_key, /*enabled*/ true) || removed_duplicate
|
||||
}
|
||||
|
||||
fn remove_other_plugin_registrations(
|
||||
doc: &mut DocumentMut,
|
||||
plugin_key: &str,
|
||||
plugin_name: &str,
|
||||
) -> bool {
|
||||
) -> Vec<String> {
|
||||
let Some(plugins) = doc
|
||||
.as_table_mut()
|
||||
.get_mut("plugins")
|
||||
.and_then(ensure_table_for_read)
|
||||
else {
|
||||
return false;
|
||||
return Vec::new();
|
||||
};
|
||||
let duplicate_keys = plugins
|
||||
.iter()
|
||||
@@ -187,9 +197,26 @@ fn remove_other_plugin_registrations(
|
||||
})
|
||||
.map(str::to_string)
|
||||
.collect::<Vec<_>>();
|
||||
let mut removed = false;
|
||||
for key in duplicate_keys {
|
||||
removed |= plugins.remove(&key).is_some();
|
||||
duplicate_keys
|
||||
.into_iter()
|
||||
.filter(|key| plugins.remove(key).is_some())
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn clear_plugin_enabled(doc: &mut DocumentMut, plugin_key: &str) -> bool {
|
||||
let Some(plugins) = doc
|
||||
.as_table_mut()
|
||||
.get_mut("plugins")
|
||||
.and_then(ensure_table_for_read)
|
||||
else {
|
||||
return false;
|
||||
};
|
||||
let Some(plugin) = plugins.get_mut(plugin_key).and_then(ensure_table_for_read) else {
|
||||
return false;
|
||||
};
|
||||
let removed = plugin.remove("enabled").is_some();
|
||||
if removed && plugin.is_empty() {
|
||||
plugins.remove(plugin_key);
|
||||
}
|
||||
removed
|
||||
}
|
||||
@@ -341,7 +368,13 @@ source = "/tmp/plugin"
|
||||
let base_path = codex_home.path().join(CONFIG_TOML_FILE);
|
||||
let profile_path = codex_home.path().join("work.config.toml");
|
||||
let base_config = "[plugins.\"structure-viewer@openai-internal-testing\"]\nenabled = true\n[plugins.\"sequence-viewer@openai-internal-testing\"]\nenabled = true\n";
|
||||
let profile_config = "[plugins.\"structure-viewer@debug\"]\nenabled = true\n";
|
||||
let profile_config = r#"[plugins."structure-viewer@debug"]
|
||||
enabled = true
|
||||
[plugins."structure-viewer@openai-monorepo"]
|
||||
enabled = false
|
||||
[plugins."structure-viewer@openai-monorepo".mcp_servers.structure]
|
||||
enabled = false
|
||||
"#;
|
||||
fs::write(&base_path, base_config).unwrap();
|
||||
fs::write(&profile_path, profile_config).unwrap();
|
||||
let layer = |file: &Path, profile: Option<&str>, config: &str| {
|
||||
@@ -362,7 +395,7 @@ source = "/tmp/plugin"
|
||||
ConfigRequirementsToml::default(),
|
||||
)
|
||||
.unwrap();
|
||||
reconcile_user_plugin_registrations(
|
||||
let removed_plugin_keys = reconcile_user_plugin_registrations(
|
||||
codex_home.path(),
|
||||
Some(&stack),
|
||||
"structure-viewer@openai-monorepo".to_string(),
|
||||
@@ -371,6 +404,13 @@ source = "/tmp/plugin"
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
removed_plugin_keys,
|
||||
vec![
|
||||
"structure-viewer@debug".to_string(),
|
||||
"structure-viewer@openai-internal-testing".to_string(),
|
||||
]
|
||||
);
|
||||
let parse = |path: &Path| {
|
||||
toml::from_str::<toml::Value>(&fs::read_to_string(path).unwrap()).unwrap()
|
||||
};
|
||||
@@ -383,7 +423,10 @@ source = "/tmp/plugin"
|
||||
);
|
||||
assert_eq!(
|
||||
parse(&profile_path),
|
||||
toml::from_str::<toml::Value>("").unwrap(),
|
||||
toml::from_str::<toml::Value>(
|
||||
"[plugins.\"structure-viewer@openai-monorepo\".mcp_servers.structure]\nenabled = false\n"
|
||||
)
|
||||
.unwrap(),
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -25,8 +25,10 @@ pub(crate) fn fingerprint_plugin_tree(
|
||||
root: &Path,
|
||||
manifest_override: Option<&str>,
|
||||
) -> Result<String, PluginStoreError> {
|
||||
let traversal_root = fs::canonicalize(root)
|
||||
.map_err(|error| PluginStoreError::io("failed to resolve plugin source", error))?;
|
||||
let mut entries = BTreeMap::new();
|
||||
for entry in WalkDir::new(root)
|
||||
for entry in WalkDir::new(&traversal_root)
|
||||
.sort_by_file_name()
|
||||
.into_iter()
|
||||
.filter_entry(|entry| entry.depth() != 1 || entry.file_name() != ".git")
|
||||
@@ -38,9 +40,12 @@ pub(crate) fn fingerprint_plugin_tree(
|
||||
std::io::Error::other(error),
|
||||
)
|
||||
})?;
|
||||
let relative = entry.path().strip_prefix(root).map_err(|error| {
|
||||
PluginStoreError::Invalid(format!("failed to fingerprint plugin path: {error}"))
|
||||
})?;
|
||||
let relative = entry
|
||||
.path()
|
||||
.strip_prefix(&traversal_root)
|
||||
.map_err(|error| {
|
||||
PluginStoreError::Invalid(format!("failed to fingerprint plugin path: {error}"))
|
||||
})?;
|
||||
let tree_entry = if entry.file_type().is_dir() {
|
||||
TreeEntry::Directory
|
||||
} else if entry.file_type().is_file() {
|
||||
@@ -69,7 +74,7 @@ pub(crate) fn fingerprint_plugin_tree(
|
||||
hash_contents(&mut hasher, path.as_os_str().as_encoded_bytes());
|
||||
match entry {
|
||||
TreeEntry::Directory => {}
|
||||
TreeEntry::File => hash_file(&mut hasher, &root.join(path))?,
|
||||
TreeEntry::File => hash_file(&mut hasher, &traversal_root.join(path))?,
|
||||
TreeEntry::VirtualFile(contents) => hash_contents(&mut hasher, &contents),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -42,6 +42,7 @@ use crate::remote::REMOTE_GLOBAL_MARKETPLACE_NAME;
|
||||
use crate::remote::RecommendedPluginsMode;
|
||||
use crate::remote::RemoteInstalledPlugin;
|
||||
use crate::remote::RemotePluginCatalogError;
|
||||
use crate::remote::RemotePluginScope;
|
||||
use crate::remote::RemotePluginServiceConfig;
|
||||
use crate::remote_legacy::RemotePluginFetchError;
|
||||
use crate::remote_legacy::RemotePluginMutationError;
|
||||
@@ -982,6 +983,7 @@ impl PluginsManager {
|
||||
self.codex_home.clone(),
|
||||
remote_plugin_service_config(config),
|
||||
auth,
|
||||
enabled_local_plugin_names(&config.config_layer_stack),
|
||||
Some(on_local_cache_changed),
|
||||
);
|
||||
}
|
||||
@@ -1515,7 +1517,7 @@ impl PluginsManager {
|
||||
config_layer_stack: Option<&ConfigLayerStack>,
|
||||
enable_canonical: bool,
|
||||
) -> Result<(), PluginInstallError> {
|
||||
reconcile_user_plugin_registrations(
|
||||
let removed_plugin_keys = reconcile_user_plugin_registrations(
|
||||
&self.codex_home,
|
||||
config_layer_stack,
|
||||
plugin_id.as_key(),
|
||||
@@ -1527,10 +1529,26 @@ impl PluginsManager {
|
||||
let store = self.store.clone();
|
||||
let canonical_plugin_id = plugin_id.clone();
|
||||
let removed_plugin_ids = tokio::task::spawn_blocking(move || {
|
||||
let removed = store.other_sources(&canonical_plugin_id, /*active_only*/ false)?;
|
||||
removed
|
||||
.iter()
|
||||
.try_for_each(|plugin_id| store.uninstall(plugin_id))?;
|
||||
let mut removed = Vec::new();
|
||||
for plugin_key in removed_plugin_keys {
|
||||
let Ok(removed_plugin_id) = PluginId::parse(&plugin_key) else {
|
||||
tracing::warn!(
|
||||
plugin = %plugin_key,
|
||||
"skipping cache cleanup for invalid reconciled plugin key"
|
||||
);
|
||||
continue;
|
||||
};
|
||||
if removed_plugin_id == canonical_plugin_id
|
||||
|| !store
|
||||
.plugin_base_root(&removed_plugin_id)
|
||||
.as_path()
|
||||
.is_dir()
|
||||
{
|
||||
continue;
|
||||
}
|
||||
store.uninstall(&removed_plugin_id)?;
|
||||
removed.push(removed_plugin_id);
|
||||
}
|
||||
Ok::<_, PluginStoreError>(removed)
|
||||
})
|
||||
.await
|
||||
@@ -2678,6 +2696,18 @@ pub(crate) fn configured_plugins_from_stack(
|
||||
configured_plugins_from_user_config_value(&user_config)
|
||||
}
|
||||
|
||||
fn enabled_local_plugin_names(config_layer_stack: &ConfigLayerStack) -> HashSet<String> {
|
||||
configured_plugins_from_stack(config_layer_stack)
|
||||
.into_iter()
|
||||
.filter_map(|(plugin_key, config)| config.enabled.then_some(plugin_key))
|
||||
.filter_map(|plugin_key| PluginId::parse(&plugin_key).ok())
|
||||
.filter(|plugin_id| {
|
||||
RemotePluginScope::from_marketplace_name(&plugin_id.marketplace_name).is_none()
|
||||
})
|
||||
.map(|plugin_id| plugin_id.plugin_name)
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn configured_plugins_from_user_config_value(
|
||||
user_config: &toml::Value,
|
||||
) -> HashMap<String, PluginConfig> {
|
||||
|
||||
@@ -2412,6 +2412,65 @@ enabled = true
|
||||
assert!(after.plugins().iter().all(|plugin| !plugin.is_active()));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn reconcile_plugin_install_preserves_inactive_profile_cache() {
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
let base_path = codex_home.path().join(CONFIG_TOML_FILE);
|
||||
let inactive_profile_path = codex_home.path().join("inactive.config.toml");
|
||||
let base_config = "[plugins.\"linear@active\"]\nenabled = true\n";
|
||||
let inactive_profile_config = "[plugins.\"linear@inactive\"]\nenabled = true\n";
|
||||
write_file(&base_path, base_config);
|
||||
write_file(&inactive_profile_path, inactive_profile_config);
|
||||
write_cached_plugin(codex_home.path(), "active", "linear");
|
||||
write_cached_plugin(codex_home.path(), "inactive", "linear");
|
||||
let stack = ConfigLayerStack::new(
|
||||
vec![ConfigLayerEntry::new(
|
||||
ConfigLayerSource::User {
|
||||
file: AbsolutePathBuf::try_from(base_path.clone()).unwrap(),
|
||||
profile: None,
|
||||
},
|
||||
toml::from_str(base_config).unwrap(),
|
||||
)],
|
||||
ConfigRequirements::default(),
|
||||
ConfigRequirementsToml::default(),
|
||||
)
|
||||
.unwrap();
|
||||
let manager = PluginsManager::new(codex_home.path().to_path_buf());
|
||||
let canonical = PluginId::new("linear".to_string(), "canonical".to_string()).unwrap();
|
||||
|
||||
manager
|
||||
.reconcile_plugin_install_locked(&canonical, Some(&stack), /*enable_canonical*/ true)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert!(
|
||||
!codex_home
|
||||
.path()
|
||||
.join("plugins/cache/active/linear")
|
||||
.exists()
|
||||
);
|
||||
assert!(
|
||||
codex_home
|
||||
.path()
|
||||
.join("plugins/cache/inactive/linear/local")
|
||||
.is_dir()
|
||||
);
|
||||
assert_eq!(
|
||||
fs::read_to_string(&inactive_profile_path).unwrap(),
|
||||
inactive_profile_config
|
||||
);
|
||||
let base: toml::Value = toml::from_str(&fs::read_to_string(base_path).unwrap()).unwrap();
|
||||
assert_eq!(
|
||||
base["plugins"]
|
||||
.as_table()
|
||||
.unwrap()
|
||||
.keys()
|
||||
.map(String::as_str)
|
||||
.collect::<Vec<_>>(),
|
||||
vec!["linear@canonical"]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn load_plugins_rejects_invalid_plugin_keys() {
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
@@ -5215,6 +5274,15 @@ fn refresh_curated_plugin_cache_reinstalls_unverified_current_version() {
|
||||
refresh_curated_plugin_cache(tmp.path(), TEST_CURATED_PLUGIN_SHA, &[plugin_id])
|
||||
.expect("cache refresh should reinstall current but unverified plugin bytes")
|
||||
);
|
||||
let plugin_id = PluginId::new(
|
||||
"slack".to_string(),
|
||||
OPENAI_CURATED_MARKETPLACE_NAME.to_string(),
|
||||
)
|
||||
.unwrap();
|
||||
assert!(
|
||||
!refresh_curated_plugin_cache(tmp.path(), TEST_CURATED_PLUGIN_SHA, &[plugin_id])
|
||||
.expect("verified current plugin bytes should be a no-op")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -414,7 +414,7 @@ impl RemotePluginScope {
|
||||
}
|
||||
}
|
||||
|
||||
fn from_marketplace_name(name: &str) -> Option<Self> {
|
||||
pub(crate) fn from_marketplace_name(name: &str) -> Option<Self> {
|
||||
match name {
|
||||
REMOTE_GLOBAL_MARKETPLACE_NAME => Some(Self::Global),
|
||||
REMOTE_CREATED_BY_ME_MARKETPLACE_NAME => Some(Self::User),
|
||||
|
||||
@@ -83,6 +83,7 @@ pub(crate) fn maybe_start_remote_installed_plugin_bundle_sync(
|
||||
codex_home: PathBuf,
|
||||
config: RemotePluginServiceConfig,
|
||||
auth: Option<CodexAuth>,
|
||||
local_competitor_plugin_names: HashSet<String>,
|
||||
on_local_cache_changed: Option<Arc<dyn Fn() + Send + Sync + 'static>>,
|
||||
) {
|
||||
let Some(auth) = auth else {
|
||||
@@ -96,8 +97,13 @@ pub(crate) fn maybe_start_remote_installed_plugin_bundle_sync(
|
||||
}
|
||||
|
||||
tokio::spawn(async move {
|
||||
let result =
|
||||
sync_remote_installed_plugin_bundles_once(codex_home, &config, Some(&auth)).await;
|
||||
let result = sync_remote_installed_plugin_bundles_once(
|
||||
codex_home,
|
||||
&config,
|
||||
Some(&auth),
|
||||
&local_competitor_plugin_names,
|
||||
)
|
||||
.await;
|
||||
match result {
|
||||
Ok(outcome) => {
|
||||
if outcome.changed_local_cache()
|
||||
@@ -127,6 +133,7 @@ pub async fn sync_remote_installed_plugin_bundles_once(
|
||||
codex_home: PathBuf,
|
||||
config: &RemotePluginServiceConfig,
|
||||
auth: Option<&CodexAuth>,
|
||||
local_competitor_plugin_names: &HashSet<String>,
|
||||
) -> Result<RemoteInstalledPluginBundleSyncOutcome, RemoteInstalledPluginBundleSyncError> {
|
||||
let auth = ensure_chatgpt_auth(auth)?;
|
||||
let global = async {
|
||||
@@ -201,7 +208,7 @@ pub async fn sync_remote_installed_plugin_bundles_once(
|
||||
continue;
|
||||
}
|
||||
};
|
||||
if has_active_local_competitor(&store, &plugin_id)? {
|
||||
if local_competitor_plugin_names.contains(&plugin.name) {
|
||||
continue;
|
||||
}
|
||||
installed_plugin_names_by_marketplace
|
||||
@@ -257,11 +264,7 @@ pub async fn sync_remote_installed_plugin_bundles_once(
|
||||
.await
|
||||
{
|
||||
Ok(result) => {
|
||||
if !has_active_local_competitor(&store, &result.plugin_id)? {
|
||||
installed_plugin_ids.insert(result.plugin_id.as_key());
|
||||
} else {
|
||||
store.uninstall(&result.plugin_id)?;
|
||||
}
|
||||
installed_plugin_ids.insert(result.plugin_id.as_key());
|
||||
}
|
||||
Err(err) => {
|
||||
warn!(
|
||||
@@ -293,15 +296,6 @@ pub async fn sync_remote_installed_plugin_bundles_once(
|
||||
})
|
||||
}
|
||||
|
||||
fn has_active_local_competitor(
|
||||
store: &PluginStore,
|
||||
plugin_id: &PluginId,
|
||||
) -> Result<bool, PluginStoreError> {
|
||||
Ok(store
|
||||
.other_sources(plugin_id, /*active_only*/ true)?
|
||||
.iter()
|
||||
.any(|other| RemotePluginScope::from_marketplace_name(&other.marketplace_name).is_none()))
|
||||
}
|
||||
pub fn mark_remote_plugin_cache_mutation_in_flight(
|
||||
codex_home: &Path,
|
||||
marketplace_name: &str,
|
||||
@@ -495,7 +489,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn sync_backfills_metadata_and_keeps_local_same_name_install_canonical() {
|
||||
async fn sync_uses_configured_local_competitors_not_stale_local_caches() {
|
||||
let server = MockServer::start().await;
|
||||
let codex_home = tempfile::tempdir().expect("create codex home");
|
||||
let cached_manifest = codex_home
|
||||
@@ -510,7 +504,13 @@ mod tests {
|
||||
.expect("create cached plugin manifest parent");
|
||||
std::fs::write(&cached_manifest, r#"{"name":"linear","version":"1.2.3"}"#)
|
||||
.expect("write cached plugin manifest");
|
||||
assert!(fs::create_dir_all(codex_home.path().join("plugins/cache/debug/linear")).is_ok());
|
||||
let local_manifest = codex_home
|
||||
.path()
|
||||
.join("plugins/cache/debug/linear/local/.codex-plugin/plugin.json");
|
||||
std::fs::create_dir_all(local_manifest.parent().expect("manifest parent"))
|
||||
.expect("create local plugin manifest parent");
|
||||
std::fs::write(&local_manifest, r#"{"name":"linear"}"#)
|
||||
.expect("write stale local plugin manifest");
|
||||
let stale_remote = codex_home
|
||||
.path()
|
||||
.join("plugins/cache/workspace-shared-with-me-private/linear/local");
|
||||
@@ -572,6 +572,7 @@ mod tests {
|
||||
codex_home.path().to_path_buf(),
|
||||
&config,
|
||||
Some(&auth),
|
||||
&HashSet::new(),
|
||||
)
|
||||
.await
|
||||
.expect("sync current remote plugin bundle");
|
||||
@@ -601,18 +602,12 @@ mod tests {
|
||||
"remote_plugin_id": remote_plugin_id,
|
||||
})
|
||||
);
|
||||
let local_manifest = codex_home
|
||||
.path()
|
||||
.join("plugins/cache/debug/linear/local/.codex-plugin/plugin.json");
|
||||
std::fs::create_dir_all(local_manifest.parent().expect("manifest parent"))
|
||||
.expect("create local plugin manifest parent");
|
||||
std::fs::write(&local_manifest, r#"{"name":"linear"}"#)
|
||||
.expect("write local plugin manifest");
|
||||
store.uninstall(&plugin_id).expect("remove remote cache");
|
||||
let outcome = sync_remote_installed_plugin_bundles_once(
|
||||
codex_home.path().to_path_buf(),
|
||||
&config,
|
||||
Some(&auth),
|
||||
&HashSet::from(["linear".to_string()]),
|
||||
)
|
||||
.await
|
||||
.expect("sync after local plugin becomes canonical");
|
||||
|
||||
@@ -386,49 +386,6 @@ impl PluginStore {
|
||||
remove_existing_target(self.plugin_base_root(plugin_id).as_path())
|
||||
}
|
||||
|
||||
pub(crate) fn other_sources(
|
||||
&self,
|
||||
canonical_plugin_id: &PluginId,
|
||||
active_only: bool,
|
||||
) -> Result<Vec<PluginId>, PluginStoreError> {
|
||||
let mut installed = Vec::new();
|
||||
let marketplaces = match fs::read_dir(self.root.as_path()) {
|
||||
Ok(entries) => entries,
|
||||
Err(err) if err.kind() == io::ErrorKind::NotFound => return Ok(installed),
|
||||
Err(err) => {
|
||||
return Err(PluginStoreError::io(
|
||||
"failed to enumerate plugin cache marketplaces",
|
||||
err,
|
||||
));
|
||||
}
|
||||
};
|
||||
for marketplace in marketplaces.filter_map(Result::ok) {
|
||||
let Ok(file_type) = marketplace.file_type() else {
|
||||
continue;
|
||||
};
|
||||
if !file_type.is_dir() {
|
||||
continue;
|
||||
}
|
||||
let Ok(marketplace_name) = marketplace.file_name().into_string() else {
|
||||
continue;
|
||||
};
|
||||
let Ok(plugin_id) =
|
||||
PluginId::new(canonical_plugin_id.plugin_name.clone(), marketplace_name)
|
||||
else {
|
||||
continue;
|
||||
};
|
||||
if &plugin_id == canonical_plugin_id
|
||||
|| (active_only && !self.is_installed(&plugin_id))
|
||||
|| (!active_only && !self.plugin_base_root(&plugin_id).as_path().is_dir())
|
||||
{
|
||||
continue;
|
||||
}
|
||||
installed.push(plugin_id);
|
||||
}
|
||||
installed.sort_unstable_by_key(PluginId::as_key);
|
||||
Ok(installed)
|
||||
}
|
||||
|
||||
fn remote_plugin_install_metadata_path(&self, plugin_id: &PluginId) -> AbsolutePathBuf {
|
||||
self.plugin_base_root(plugin_id)
|
||||
.join(REMOTE_PLUGIN_INSTALL_METADATA_FILE)
|
||||
|
||||
@@ -222,6 +222,32 @@ fn install_records_and_verifies_exact_source_bytes() {
|
||||
fs::write(source.join("skills/SKILL.md"), "updated source").expect("update source file");
|
||||
assert!(!matches_source());
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[cfg(unix)]
|
||||
fn install_accepts_symlinked_plugin_source_root() {
|
||||
use std::os::unix::fs::symlink;
|
||||
|
||||
let tmp = tempdir().unwrap();
|
||||
write_plugin(tmp.path(), "real-plugin", "sample-plugin");
|
||||
let linked_source = tmp.path().join("linked-plugin");
|
||||
symlink(tmp.path().join("real-plugin"), &linked_source).unwrap();
|
||||
let plugin_id = PluginId::new("sample-plugin".to_string(), "debug".to_string()).unwrap();
|
||||
let store = PluginStore::new(tmp.path().to_path_buf());
|
||||
let source = AbsolutePathBuf::try_from(linked_source).unwrap();
|
||||
|
||||
let result = store
|
||||
.install(source.clone(), plugin_id.clone())
|
||||
.expect("install plugin through symlinked source root");
|
||||
|
||||
assert!(result.installed_path.join("skills/SKILL.md").is_file());
|
||||
assert!(
|
||||
store
|
||||
.active_plugin_matches_source(&plugin_id, source.as_path())
|
||||
.expect("verify symlinked source bytes")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn remote_plugin_install_metadata_follows_installed_cache_lifecycle() {
|
||||
let tmp = tempdir().unwrap();
|
||||
|
||||
@@ -95,6 +95,41 @@ fn write_plugin_mcp_plugin(home: &TempDir, command: &str) {
|
||||
.expect("write plugin mcp config");
|
||||
}
|
||||
|
||||
fn write_installed_structure_plugin(
|
||||
home: &TempDir,
|
||||
marketplace: &str,
|
||||
skill_name: &str,
|
||||
server_name: &str,
|
||||
command: &str,
|
||||
) {
|
||||
let plugin_root = home
|
||||
.path()
|
||||
.join("plugins/cache")
|
||||
.join(marketplace)
|
||||
.join("structure-viewer/local");
|
||||
std::fs::create_dir_all(plugin_root.join(".codex-plugin"))
|
||||
.expect("create installed plugin manifest dir");
|
||||
std::fs::write(
|
||||
plugin_root.join(".codex-plugin/plugin.json"),
|
||||
r#"{"name":"structure-viewer","description":"View structures"}"#,
|
||||
)
|
||||
.expect("write installed plugin manifest");
|
||||
let skill_root = plugin_root.join("skills").join(skill_name);
|
||||
std::fs::create_dir_all(&skill_root).expect("create installed plugin skill dir");
|
||||
std::fs::write(
|
||||
skill_root.join("SKILL.md"),
|
||||
format!("---\nname: {skill_name}\ndescription: {skill_name} structure skill\n---\n"),
|
||||
)
|
||||
.expect("write installed plugin skill");
|
||||
std::fs::write(
|
||||
plugin_root.join(".mcp.json"),
|
||||
format!(
|
||||
r#"{{"mcpServers":{{"{server_name}":{{"command":"{command}","cwd":".","startup_timeout_sec":60.0}}}}}}"#
|
||||
),
|
||||
)
|
||||
.expect("write installed plugin MCP config");
|
||||
}
|
||||
|
||||
fn write_plugin_app_plugin(home: &TempDir) {
|
||||
write_plugin_app_plugin_with_name(home, "sample");
|
||||
}
|
||||
@@ -274,6 +309,70 @@ async fn capability_sections_render_in_developer_message_in_order() -> Result<()
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn agent_context_uses_only_configured_same_name_plugin_install() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
let server = start_mock_server().await;
|
||||
let response_mock = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![ev_response_created("resp1"), ev_completed("resp1")]),
|
||||
)
|
||||
.await;
|
||||
let rmcp_test_server_bin = match stdio_server_bin() {
|
||||
Ok(bin) => bin,
|
||||
Err(err) => {
|
||||
eprintln!("test_stdio_server binary not available, skipping test: {err}");
|
||||
return Ok(());
|
||||
}
|
||||
};
|
||||
let codex_home = Arc::new(TempDir::new()?);
|
||||
write_installed_structure_plugin(
|
||||
codex_home.as_ref(),
|
||||
"openai-monorepo",
|
||||
"canonical",
|
||||
"canonical",
|
||||
&rmcp_test_server_bin,
|
||||
);
|
||||
write_installed_structure_plugin(
|
||||
codex_home.as_ref(),
|
||||
"openai-internal-testing",
|
||||
"stale",
|
||||
"stale",
|
||||
&rmcp_test_server_bin,
|
||||
);
|
||||
std::fs::write(
|
||||
codex_home.path().join("config.toml"),
|
||||
"[features]\nplugins = true\n[plugins.\"structure-viewer@openai-monorepo\"]\nenabled = true\n",
|
||||
)?;
|
||||
let mut builder = test_codex()
|
||||
.with_home(codex_home)
|
||||
.with_auth(CodexAuth::from_api_key("Test API Key"));
|
||||
let codex = builder.build(&server).await?.codex;
|
||||
wait_for_mcp_server(&codex, "canonical").await?;
|
||||
|
||||
codex
|
||||
.submit(Op::UserInput {
|
||||
items: vec![codex_protocol::user_input::UserInput::Text {
|
||||
text: "inspect the structure".into(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
responsesapi_client_metadata: None,
|
||||
additional_context: Default::default(),
|
||||
thread_settings: Default::default(),
|
||||
})
|
||||
.await?;
|
||||
wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
|
||||
|
||||
let request = response_mock.single_request();
|
||||
let developer_text = request.message_input_texts("developer").join("\n");
|
||||
assert!(developer_text.contains("structure-viewer:canonical: canonical structure skill"));
|
||||
assert!(!developer_text.contains("stale structure skill"));
|
||||
assert!(request.tool_by_name("mcp__canonical", "echo").is_some());
|
||||
assert!(request.tool_by_name("mcp__stale", "echo").is_none());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn explicit_plugin_mentions_use_apps_for_chatgpt_dual_surface_plugins() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
Reference in New Issue
Block a user