mirror of
https://github.com/openai/codex.git
synced 2026-11-01 19:33:35 +00:00
Honor canonical plugin disables for shared connectors (#45755)
## Why Disabling a connector's canonical plugin could leave its tools available when another enabled plugin contributed the same connector. ## What changed Retain `canonical_app_id` in the remote installed-plugin cache and use ownership metadata for the current account when building connector snapshots. A disabled canonical owner now excludes its connector even when another plugin contributes it or the owner's bundle is absent from the host. Match owners by plugin name and marketplace, and ignore ownership metadata when the cache no longer matches the current authentication. Build these snapshots through `PluginsManager` when the plugins feature is enabled; otherwise use an empty snapshot. ## Testing Add manager coverage for combined local and canonical exclusions, marketplace matching, and authentication changes. Add an integration test showing that disabling a noncanonical contributor preserves shared calendar tools, disabling the canonical owner hides them, and clearing the exclusion restores them. GitOrigin-RevId: ee2981d1277825fefb671dd7d1078cd7ea5c0ea5
This commit is contained in:
@@ -99,6 +99,8 @@ use codex_config::skill_config_rules_from_stack;
|
||||
use codex_config::types::PluginConfig;
|
||||
use codex_config::types::ToolSuggestDisabledTool;
|
||||
use codex_config::types::ToolSuggestDiscoverableType;
|
||||
use codex_connectors::ConnectorSnapshot;
|
||||
use codex_connectors::PluginConnectorSource;
|
||||
use codex_hooks::plugin_hook_declarations;
|
||||
use codex_http_client::HttpClientFactory;
|
||||
use codex_login::AuthManager;
|
||||
@@ -1024,6 +1026,40 @@ impl PluginsManager {
|
||||
}
|
||||
}
|
||||
|
||||
/// Applies plugin exclusions and canonical ownership from the current account's installed cache.
|
||||
/// A canonical owner's bundle need not be installed on this host.
|
||||
pub fn connector_snapshot(
|
||||
&self,
|
||||
sources: impl IntoIterator<Item = PluginConnectorSource>,
|
||||
disabled_plugin_ids: &[String],
|
||||
) -> ConnectorSnapshot {
|
||||
let mut canonical_app_ids = HashSet::new();
|
||||
if !disabled_plugin_ids.is_empty() {
|
||||
let cache = match self.remote_installed_plugins_cache.read() {
|
||||
Ok(cache) => cache,
|
||||
Err(err) => err.into_inner(),
|
||||
};
|
||||
if self.remote_installed_plugins_cache_matches_current_auth(&cache) {
|
||||
canonical_app_ids = cache
|
||||
.plugins
|
||||
.as_deref()
|
||||
.unwrap_or_default()
|
||||
.iter()
|
||||
.filter_map(|plugin| {
|
||||
let app_id = plugin.canonical_app_id.as_ref()?;
|
||||
let plugin_id =
|
||||
PluginId::new(plugin.name.clone(), plugin.marketplace_name.clone())
|
||||
.ok()?;
|
||||
disabled_plugin_ids
|
||||
.contains(&plugin_id.as_key())
|
||||
.then(|| app_id.clone())
|
||||
})
|
||||
.collect();
|
||||
}
|
||||
}
|
||||
ConnectorSnapshot::from_plugin_sources(sources, disabled_plugin_ids, canonical_app_ids)
|
||||
}
|
||||
|
||||
fn remote_installed_plugin_configs(&self) -> HashMap<String, PluginConfig> {
|
||||
let cache = match self.remote_installed_plugins_cache.read() {
|
||||
Ok(cache) => cache,
|
||||
|
||||
@@ -864,6 +864,7 @@ fn remote_installed_plugin_in_marketplace(
|
||||
marketplace_name: &str,
|
||||
) -> RemoteInstalledPlugin {
|
||||
RemoteInstalledPlugin {
|
||||
canonical_app_id: None,
|
||||
marketplace_name: marketplace_name.to_string(),
|
||||
id: format!("plugins~Plugin_{name}"),
|
||||
version: None,
|
||||
@@ -3102,6 +3103,55 @@ enabled = true
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn connector_snapshot_combines_plugin_exclusions_with_current_account_ownership() {
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
let auth_manager = test_auth_manager(Some(AuthMode::Chatgpt));
|
||||
let manager = test_plugins_manager_with_auth_manager(
|
||||
codex_home.path().to_path_buf(),
|
||||
Some(Product::Codex),
|
||||
Arc::clone(&auth_manager),
|
||||
);
|
||||
let sources = [PluginConnectorSource::from_connector_ids(
|
||||
"local@test",
|
||||
"Local",
|
||||
[AppConnectorId("local-connector".to_string())],
|
||||
)];
|
||||
let disabled = vec![
|
||||
"linear@openai-curated-remote".to_string(),
|
||||
"local@test".to_string(),
|
||||
];
|
||||
let local_exclusion = HashSet::from(["local-connector".to_string()]);
|
||||
assert_eq!(
|
||||
manager
|
||||
.connector_snapshot(sources.clone(), &disabled)
|
||||
.disabled_connector_ids(),
|
||||
&local_exclusion,
|
||||
);
|
||||
let mut plugin = remote_installed_linear_plugin();
|
||||
plugin.canonical_app_id = Some("linear".to_string());
|
||||
manager.write_remote_installed_plugins_cache(vec![plugin]);
|
||||
assert_eq!(
|
||||
manager
|
||||
.connector_snapshot(sources.clone(), &disabled)
|
||||
.disabled_connector_ids(),
|
||||
&HashSet::from(["linear".to_string(), "local-connector".to_string()]),
|
||||
);
|
||||
assert!(
|
||||
manager
|
||||
.connector_snapshot(sources.clone(), &["linear@another-marketplace".to_string()])
|
||||
.disabled_connector_ids()
|
||||
.is_empty()
|
||||
);
|
||||
set_test_auth_mode(&auth_manager, Some(AuthMode::ApiKey)).await;
|
||||
assert_eq!(
|
||||
manager
|
||||
.connector_snapshot(sources, &disabled)
|
||||
.disabled_connector_ids(),
|
||||
&local_exclusion,
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn loaded_plugins_cache_evicts_least_recently_used_configuration() {
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
|
||||
@@ -230,6 +230,7 @@ pub struct RemoteInstalledPlugin {
|
||||
pub id: String,
|
||||
pub version: Option<String>,
|
||||
pub name: String,
|
||||
pub canonical_app_id: Option<String>,
|
||||
pub installed_at: Option<DateTime<Utc>>,
|
||||
pub enabled: bool,
|
||||
pub install_policy: PluginInstallPolicy,
|
||||
@@ -682,6 +683,8 @@ impl RemotePluginInstallPolicySource {
|
||||
struct RemotePluginDirectoryItem {
|
||||
id: String,
|
||||
name: String,
|
||||
#[serde(default)]
|
||||
canonical_app_id: Option<String>,
|
||||
scope: RemotePluginScope,
|
||||
#[serde(default)]
|
||||
discoverability: Option<RemotePluginShareDiscoverability>,
|
||||
@@ -1879,6 +1882,7 @@ fn remote_installed_plugin_to_cache_entry(
|
||||
id: plugin.id.clone(),
|
||||
version: plugin.release.version.clone(),
|
||||
name: plugin.name.clone(),
|
||||
canonical_app_id: plugin.canonical_app_id.clone(),
|
||||
installed_at: installed_plugin.installed_at,
|
||||
enabled: installed_plugin.enabled,
|
||||
install_policy: plugin.installation_policy,
|
||||
|
||||
@@ -596,6 +596,7 @@ fn workspace_share_context_preserves_publish_capability() {
|
||||
|
||||
fn directory_plugin(id: &str, name: &str) -> RemotePluginDirectoryItem {
|
||||
RemotePluginDirectoryItem {
|
||||
canonical_app_id: None,
|
||||
id: id.to_string(),
|
||||
name: name.to_string(),
|
||||
scope: RemotePluginScope::Global,
|
||||
|
||||
Reference in New Issue
Block a user