mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
feat: Normalize remote plugin summary identities. (#22265)
Makes plugin summaries use config-style plugin@marketplace IDs while exposing backend remote IDs separately as remotePluginId. Also fix the consistency issue of REMOTE_SHARED_WITH_ME_MARKETPLACE_NAME
This commit is contained in:
committed by
GitHub
Unverified
parent
46f30d0282
commit
5b1a4c2fa7
@@ -87,6 +87,7 @@ pub struct RemoteInstalledPlugin {
|
||||
#[derive(Debug, Clone, PartialEq)]
|
||||
pub struct RemotePluginSummary {
|
||||
pub id: String,
|
||||
pub remote_plugin_id: String,
|
||||
pub name: String,
|
||||
pub share_context: Option<RemotePluginShareContext>,
|
||||
pub installed: bool,
|
||||
@@ -379,6 +380,32 @@ struct RemotePluginDirectoryItem {
|
||||
release: RemotePluginReleaseResponse,
|
||||
}
|
||||
|
||||
fn remote_plugin_canonical_marketplace_name(
|
||||
plugin: &RemotePluginDirectoryItem,
|
||||
) -> Result<&'static str, RemotePluginCatalogError> {
|
||||
match plugin.scope {
|
||||
RemotePluginScope::Global => Ok(REMOTE_GLOBAL_MARKETPLACE_NAME),
|
||||
RemotePluginScope::Workspace => match workspace_plugin_discoverability(plugin)? {
|
||||
RemotePluginShareDiscoverability::Listed => Ok(REMOTE_WORKSPACE_MARKETPLACE_NAME),
|
||||
RemotePluginShareDiscoverability::Unlisted
|
||||
| RemotePluginShareDiscoverability::Private => {
|
||||
Ok(REMOTE_SHARED_WITH_ME_MARKETPLACE_NAME)
|
||||
}
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
fn workspace_plugin_discoverability(
|
||||
plugin: &RemotePluginDirectoryItem,
|
||||
) -> Result<RemotePluginShareDiscoverability, RemotePluginCatalogError> {
|
||||
plugin.discoverability.ok_or_else(|| {
|
||||
RemotePluginCatalogError::UnexpectedResponse(format!(
|
||||
"workspace plugin `{}` did not include discoverability",
|
||||
plugin.id
|
||||
))
|
||||
})
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Deserialize)]
|
||||
struct RemotePluginDirectorySharePrincipal {
|
||||
principal_type: RemotePluginSharePrincipalType,
|
||||
@@ -550,12 +577,9 @@ pub async fn fetch_remote_installed_plugins(
|
||||
let (global, workspace) = tokio::try_join!(global, workspace)?;
|
||||
let mut installed_plugins = [global, workspace]
|
||||
.into_iter()
|
||||
.flat_map(|(scope, plugins)| {
|
||||
plugins
|
||||
.into_iter()
|
||||
.map(move |plugin| remote_installed_plugin_to_info(scope, &plugin))
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
.flat_map(|(_scope, plugins)| plugins)
|
||||
.map(|plugin| remote_installed_plugin_to_info(&plugin))
|
||||
.collect::<Result<Vec<_>, _>>()?;
|
||||
installed_plugins.sort_by(|left, right| {
|
||||
left.marketplace_name
|
||||
.cmp(&right.marketplace_name)
|
||||
@@ -655,7 +679,7 @@ async fn fetch_remote_plugin_detail_with_download_url_option(
|
||||
let auth = ensure_chatgpt_auth(auth)?;
|
||||
let plugin = fetch_plugin_detail(config, auth, plugin_id, include_download_urls).await?;
|
||||
let scope = plugin.scope;
|
||||
let marketplace_name = scope.marketplace_name().to_string();
|
||||
let marketplace_name = remote_plugin_canonical_marketplace_name(&plugin)?.to_string();
|
||||
// Remote plugin IDs uniquely identify remote plugins, so the caller-provided
|
||||
// marketplace name is not validated here. The backend detail response is the
|
||||
// source of truth for the plugin's actual scope/marketplace.
|
||||
@@ -756,7 +780,7 @@ pub async fn uninstall_remote_plugin(
|
||||
config, auth, plugin_id, /*include_download_urls*/ false,
|
||||
)
|
||||
.await?;
|
||||
let marketplace_name = plugin.scope.marketplace_name().to_string();
|
||||
let marketplace_name = remote_plugin_canonical_marketplace_name(&plugin)?.to_string();
|
||||
let plugin_name = plugin.name;
|
||||
|
||||
let base_url = config.chatgpt_base_url.trim_end_matches('/');
|
||||
@@ -841,8 +865,17 @@ fn build_remote_plugin_summary(
|
||||
plugin: &RemotePluginDirectoryItem,
|
||||
installed_plugin: Option<&RemotePluginInstalledItem>,
|
||||
) -> Result<RemotePluginSummary, RemotePluginCatalogError> {
|
||||
let marketplace_name = remote_plugin_canonical_marketplace_name(plugin)?;
|
||||
let plugin_id =
|
||||
PluginId::new(plugin.name.clone(), marketplace_name.to_string()).map_err(|err| {
|
||||
RemotePluginCatalogError::UnexpectedResponse(format!(
|
||||
"invalid remote plugin config id for `{}` in `{marketplace_name}`: {err}",
|
||||
plugin.name
|
||||
))
|
||||
})?;
|
||||
Ok(RemotePluginSummary {
|
||||
id: plugin.id.clone(),
|
||||
id: plugin_id.as_key(),
|
||||
remote_plugin_id: plugin.id.clone(),
|
||||
name: plugin.name.clone(),
|
||||
share_context: remote_plugin_share_context(plugin)?,
|
||||
installed: installed_plugin.is_some(),
|
||||
@@ -861,12 +894,7 @@ fn remote_plugin_share_context(
|
||||
match plugin.scope {
|
||||
RemotePluginScope::Global => Ok(None),
|
||||
RemotePluginScope::Workspace => {
|
||||
let discoverability = plugin.discoverability.ok_or_else(|| {
|
||||
RemotePluginCatalogError::UnexpectedResponse(format!(
|
||||
"workspace plugin `{}` did not include discoverability",
|
||||
plugin.id
|
||||
))
|
||||
})?;
|
||||
let discoverability = workspace_plugin_discoverability(plugin)?;
|
||||
Ok(Some(RemotePluginShareContext {
|
||||
remote_plugin_id: plugin.id.clone(),
|
||||
discoverability,
|
||||
@@ -890,19 +918,18 @@ fn remote_plugin_share_context(
|
||||
}
|
||||
|
||||
fn remote_installed_plugin_to_info(
|
||||
scope: RemotePluginScope,
|
||||
installed_plugin: &RemotePluginInstalledItem,
|
||||
) -> RemoteInstalledPlugin {
|
||||
) -> Result<RemoteInstalledPlugin, RemotePluginCatalogError> {
|
||||
let plugin = &installed_plugin.plugin;
|
||||
// Remote per-skill disabled state (`disabled_skill_names`) is intentionally
|
||||
// not projected into skills/list yet; local skills.config remains the
|
||||
// supported source for skill enablement.
|
||||
RemoteInstalledPlugin {
|
||||
marketplace_name: scope.marketplace_name().to_string(),
|
||||
Ok(RemoteInstalledPlugin {
|
||||
marketplace_name: remote_plugin_canonical_marketplace_name(plugin)?.to_string(),
|
||||
id: plugin.id.clone(),
|
||||
name: plugin.name.clone(),
|
||||
enabled: installed_plugin.enabled,
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
fn remote_plugin_interface_to_info(plugin: &RemotePluginDirectoryItem) -> Option<PluginInterface> {
|
||||
|
||||
@@ -1,10 +1,12 @@
|
||||
use super::REMOTE_GLOBAL_MARKETPLACE_NAME;
|
||||
use super::REMOTE_SHARED_WITH_ME_MARKETPLACE_NAME;
|
||||
use super::REMOTE_WORKSPACE_MARKETPLACE_NAME;
|
||||
use super::RemotePluginCatalogError;
|
||||
use super::RemotePluginScope;
|
||||
use super::RemotePluginServiceConfig;
|
||||
use super::ensure_chatgpt_auth;
|
||||
use super::fetch_installed_plugins_for_scope_with_download_url;
|
||||
use super::remote_plugin_canonical_marketplace_name;
|
||||
use crate::store::PLUGINS_CACHE_DIR;
|
||||
use crate::store::PluginStore;
|
||||
use crate::store::PluginStoreError;
|
||||
@@ -150,14 +152,18 @@ pub async fn sync_remote_installed_plugin_bundles_once(
|
||||
REMOTE_WORKSPACE_MARKETPLACE_NAME.to_string(),
|
||||
BTreeSet::new(),
|
||||
),
|
||||
(
|
||||
REMOTE_SHARED_WITH_ME_MARKETPLACE_NAME.to_string(),
|
||||
BTreeSet::new(),
|
||||
),
|
||||
]);
|
||||
let mut installed_plugin_ids = BTreeSet::new();
|
||||
let mut failed_remote_plugin_ids = BTreeSet::new();
|
||||
|
||||
for (scope, installed_plugins) in [global, workspace] {
|
||||
let marketplace_name = scope.marketplace_name().to_string();
|
||||
for (_scope, installed_plugins) in [global, workspace] {
|
||||
for installed_plugin in installed_plugins {
|
||||
let plugin = installed_plugin.plugin;
|
||||
let marketplace_name = remote_plugin_canonical_marketplace_name(&plugin)?.to_string();
|
||||
installed_plugin_names_by_marketplace
|
||||
.entry(marketplace_name.clone())
|
||||
.or_default()
|
||||
@@ -292,6 +298,7 @@ fn remove_stale_remote_plugin_caches(
|
||||
for marketplace_name in [
|
||||
REMOTE_GLOBAL_MARKETPLACE_NAME,
|
||||
REMOTE_WORKSPACE_MARKETPLACE_NAME,
|
||||
REMOTE_SHARED_WITH_ME_MARKETPLACE_NAME,
|
||||
] {
|
||||
let marketplace_root = codex_home.join(PLUGINS_CACHE_DIR).join(marketplace_name);
|
||||
if !marketplace_root.exists() {
|
||||
@@ -449,6 +456,10 @@ mod tests {
|
||||
REMOTE_WORKSPACE_MARKETPLACE_NAME.to_string(),
|
||||
BTreeSet::new(),
|
||||
),
|
||||
(
|
||||
REMOTE_SHARED_WITH_ME_MARKETPLACE_NAME.to_string(),
|
||||
BTreeSet::new(),
|
||||
),
|
||||
]);
|
||||
|
||||
let guard = mark_remote_plugin_cache_mutation_in_flight(
|
||||
@@ -487,4 +498,42 @@ mod tests {
|
||||
assert_eq!(removed, vec!["linear@chatgpt-global".to_string()]);
|
||||
assert!(!cached_manifest.exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn stale_remote_plugin_cleanup_removes_shared_with_me_cache() {
|
||||
let codex_home = tempfile::tempdir().expect("create codex home");
|
||||
let cached_manifest = codex_home
|
||||
.path()
|
||||
.join(PLUGINS_CACHE_DIR)
|
||||
.join(REMOTE_SHARED_WITH_ME_MARKETPLACE_NAME)
|
||||
.join("private-plugin")
|
||||
.join("1.2.3")
|
||||
.join(".codex-plugin")
|
||||
.join("plugin.json");
|
||||
std::fs::create_dir_all(cached_manifest.parent().expect("manifest parent"))
|
||||
.expect("create cached plugin manifest parent");
|
||||
std::fs::write(&cached_manifest, r#"{"name":"private-plugin"}"#)
|
||||
.expect("write cached plugin manifest");
|
||||
let installed_plugin_names_by_marketplace =
|
||||
BTreeMap::<String, BTreeSet<String>>::from_iter([
|
||||
(REMOTE_GLOBAL_MARKETPLACE_NAME.to_string(), BTreeSet::new()),
|
||||
(
|
||||
REMOTE_WORKSPACE_MARKETPLACE_NAME.to_string(),
|
||||
BTreeSet::new(),
|
||||
),
|
||||
(
|
||||
REMOTE_SHARED_WITH_ME_MARKETPLACE_NAME.to_string(),
|
||||
BTreeSet::new(),
|
||||
),
|
||||
]);
|
||||
|
||||
let removed = remove_stale_remote_plugin_caches(
|
||||
codex_home.path(),
|
||||
&installed_plugin_names_by_marketplace,
|
||||
)
|
||||
.expect("cleanup shared-with-me cache");
|
||||
|
||||
assert_eq!(removed, vec!["private-plugin@shared-with-me".to_string()]);
|
||||
assert!(!cached_manifest.exists());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -93,6 +93,7 @@ fn remote_plugin_json(plugin_id: &str) -> serde_json::Value {
|
||||
"id": plugin_id,
|
||||
"name": "demo-plugin",
|
||||
"scope": "WORKSPACE",
|
||||
"discoverability": "PRIVATE",
|
||||
"installation_policy": "AVAILABLE",
|
||||
"authentication_policy": "ON_USE",
|
||||
"release": {
|
||||
@@ -584,7 +585,8 @@ async fn list_remote_plugin_shares_fetches_created_workspace_plugins() {
|
||||
vec![
|
||||
RemotePluginShareSummary {
|
||||
summary: RemotePluginSummary {
|
||||
id: "plugins_123".to_string(),
|
||||
id: "demo-plugin@shared-with-me".to_string(),
|
||||
remote_plugin_id: "plugins_123".to_string(),
|
||||
name: "demo-plugin".to_string(),
|
||||
share_context: Some(RemotePluginShareContext {
|
||||
remote_plugin_id: "plugins_123".to_string(),
|
||||
@@ -621,7 +623,8 @@ async fn list_remote_plugin_shares_fetches_created_workspace_plugins() {
|
||||
},
|
||||
RemotePluginShareSummary {
|
||||
summary: RemotePluginSummary {
|
||||
id: "plugins_456".to_string(),
|
||||
id: "demo-plugin@shared-with-me".to_string(),
|
||||
remote_plugin_id: "plugins_456".to_string(),
|
||||
name: "demo-plugin".to_string(),
|
||||
share_context: Some(RemotePluginShareContext {
|
||||
remote_plugin_id: "plugins_456".to_string(),
|
||||
|
||||
Reference in New Issue
Block a user