[codex] Canonicalize shared workspace plugin IDs (#22564)

## Summary
- Canonicalize private and unlisted workspace shared plugin IDs to
`workspace-shared-with-me`.
- Keep `plugin/list` private/unlisted shared-with-me buckets as UI
grouping only.
- Update share read/list/checkout and cache cleanup coverage for the
canonical namespace.

## Tests
- `cargo test -p codex-app-server --test all
plugin_list_fetches_shared_with_me_kind`
- `cargo test -p codex-app-server --test all
plugin_read_returns_share_context_for_shared_remote_plugin`
- `cargo test -p codex-app-server --test all suite::v2::plugin_share`
- `cargo test -p codex-core-plugins
list_remote_plugin_shares_fetches_created_workspace_plugins`
- `cargo test -p codex-core-plugins
stale_remote_plugin_cleanup_removes_old_shared_with_me_cache_and_keeps_canonical_cache`
- `git diff --check`
This commit is contained in:
xl-openai
2026-05-13 16:29:47 -07:00
committed by GitHub
parent 3c3e18c222
commit e3bf0cfc63
7 changed files with 164 additions and 88 deletions

View File

@@ -1823,6 +1823,18 @@ async fn plugin_list_fetches_shared_with_me_kind() -> Result<()> {
/*enabled*/ None,
))?;
shared_plugin_body["plugins"][0]["share_principals"] = serde_json::Value::Null;
let shared_unlisted_body: serde_json::Value =
serde_json::from_str(&workspace_remote_plugin_page_body(
"plugins~Plugin_44444444444444444444444444444444",
"shared-unlisted-linear",
"Shared Unlisted Linear",
"UNLISTED",
/*enabled*/ None,
))?;
shared_plugin_body["plugins"]
.as_array_mut()
.expect("shared plugins should be an array")
.push(shared_unlisted_body["plugins"][0].clone());
let shared_plugin_body = serde_json::to_string(&shared_plugin_body)?;
let mut workspace_installed_body: serde_json::Value =
serde_json::from_str(&workspace_remote_plugin_page_body(
@@ -1878,10 +1890,10 @@ async fn plugin_list_fetches_shared_with_me_kind() -> Result<()> {
.and_then(|interface| interface.display_name.as_deref()),
Some("Shared with me")
);
assert_eq!(marketplace.plugins.len(), 1);
assert_eq!(marketplace.plugins.len(), 2);
assert_eq!(
marketplace.plugins[0].id,
"shared-linear@workspace-shared-with-me-private"
"shared-linear@workspace-shared-with-me"
);
assert_eq!(
marketplace.plugins[0].remote_plugin_id.as_deref(),
@@ -1913,6 +1925,29 @@ async fn plugin_list_fetches_shared_with_me_kind() -> Result<()> {
Some("https://chatgpt.example/plugins/share/share-key-1")
);
assert_eq!(share_context.share_principals, None);
assert_eq!(
marketplace.plugins[1].id,
"shared-unlisted-linear@workspace-shared-with-me"
);
assert_eq!(
marketplace.plugins[1].remote_plugin_id.as_deref(),
Some("plugins~Plugin_44444444444444444444444444444444")
);
assert_eq!(marketplace.plugins[1].name, "shared-unlisted-linear");
assert_eq!(marketplace.plugins[1].installed, false);
assert_eq!(marketplace.plugins[1].enabled, false);
let share_context = marketplace.plugins[1]
.share_context
.as_ref()
.expect("expected share context");
assert_eq!(
share_context.remote_plugin_id,
"plugins~Plugin_44444444444444444444444444444444"
);
assert_eq!(
share_context.discoverability,
Some(PluginShareDiscoverability::Unlisted)
);
let marketplace = response
.marketplaces
@@ -1929,7 +1964,7 @@ async fn plugin_list_fetches_shared_with_me_kind() -> Result<()> {
assert_eq!(marketplace.plugins.len(), 1);
assert_eq!(
marketplace.plugins[0].id,
"unlisted-linear@workspace-shared-with-me-unlisted"
"unlisted-linear@workspace-shared-with-me"
);
assert_eq!(
marketplace.plugins[0].remote_plugin_id.as_deref(),

View File

@@ -300,74 +300,76 @@ async fn plugin_read_returns_share_context_for_shared_remote_plugin() -> Result<
let mut mcp = McpProcess::new(codex_home.path()).await?;
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
let request_id = mcp
.send_plugin_read_request(PluginReadParams {
marketplace_path: None,
remote_marketplace_name: Some("workspace-shared-with-me-private".to_string()),
plugin_name: "plugins~Plugin_11111111111111111111111111111111".to_string(),
})
.await?;
for remote_marketplace_name in [
"workspace-shared-with-me-private",
"workspace-shared-with-me",
] {
let request_id = mcp
.send_plugin_read_request(PluginReadParams {
marketplace_path: None,
remote_marketplace_name: Some(remote_marketplace_name.to_string()),
plugin_name: "plugins~Plugin_11111111111111111111111111111111".to_string(),
})
.await?;
let response: JSONRPCResponse = timeout(
DEFAULT_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
)
.await??;
let response: PluginReadResponse = to_response(response)?;
let response: JSONRPCResponse = timeout(
DEFAULT_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
)
.await??;
let response: PluginReadResponse = to_response(response)?;
assert_eq!(
response.plugin.marketplace_name,
"workspace-shared-with-me-private"
);
assert_eq!(
response.plugin.summary.id,
"shared-linear@workspace-shared-with-me-private"
);
assert_eq!(
response.plugin.summary.remote_plugin_id.as_deref(),
Some("plugins~Plugin_11111111111111111111111111111111")
);
let share_context = response
.plugin
.summary
.share_context
.as_ref()
.expect("expected share context");
assert_eq!(
share_context.remote_plugin_id,
"plugins~Plugin_11111111111111111111111111111111"
);
assert_eq!(share_context.remote_version.as_deref(), Some("2.3.4"));
assert_eq!(
share_context.discoverability,
Some(PluginShareDiscoverability::Private)
);
assert_eq!(
share_context.creator_account_user_id.as_deref(),
Some("user-gavin__account-123")
);
assert_eq!(share_context.creator_name.as_deref(), Some("Gavin"));
assert_eq!(
share_context.share_url.as_deref(),
Some("https://chatgpt.example/plugins/share/share-key-1")
);
assert_eq!(
share_context.share_principals,
Some(vec![
PluginSharePrincipal {
principal_type: PluginSharePrincipalType::User,
principal_id: "user-gavin__account-123".to_string(),
role: PluginSharePrincipalRole::Owner,
name: "Gavin".to_string(),
},
PluginSharePrincipal {
principal_type: PluginSharePrincipalType::User,
principal_id: "user-ada__account-123".to_string(),
role: PluginSharePrincipalRole::Reader,
name: "Ada".to_string(),
},
])
);
assert_eq!(response.plugin.marketplace_name, "workspace-shared-with-me");
assert_eq!(
response.plugin.summary.id,
"shared-linear@workspace-shared-with-me"
);
assert_eq!(
response.plugin.summary.remote_plugin_id.as_deref(),
Some("plugins~Plugin_11111111111111111111111111111111")
);
let share_context = response
.plugin
.summary
.share_context
.as_ref()
.expect("expected share context");
assert_eq!(
share_context.remote_plugin_id,
"plugins~Plugin_11111111111111111111111111111111"
);
assert_eq!(share_context.remote_version.as_deref(), Some("2.3.4"));
assert_eq!(
share_context.discoverability,
Some(PluginShareDiscoverability::Private)
);
assert_eq!(
share_context.creator_account_user_id.as_deref(),
Some("user-gavin__account-123")
);
assert_eq!(share_context.creator_name.as_deref(), Some("Gavin"));
assert_eq!(
share_context.share_url.as_deref(),
Some("https://chatgpt.example/plugins/share/share-key-1")
);
assert_eq!(
share_context.share_principals,
Some(vec![
PluginSharePrincipal {
principal_type: PluginSharePrincipalType::User,
principal_id: "user-gavin__account-123".to_string(),
role: PluginSharePrincipalRole::Owner,
name: "Gavin".to_string(),
},
PluginSharePrincipal {
principal_type: PluginSharePrincipalType::User,
principal_id: "user-ada__account-123".to_string(),
role: PluginSharePrincipalRole::Reader,
name: "Ada".to_string(),
},
])
);
}
Ok(())
}

View File

@@ -169,7 +169,7 @@ async fn plugin_share_save_uploads_local_plugin() -> Result<()> {
PluginShareListResponse {
data: vec![PluginShareListItem {
plugin: PluginSummary {
id: "demo-plugin@workspace-shared-with-me-private".to_string(),
id: "demo-plugin@workspace-shared-with-me".to_string(),
remote_plugin_id: Some("plugins_123".to_string()),
local_version: None,
name: "demo-plugin".to_string(),
@@ -573,7 +573,7 @@ async fn plugin_share_list_returns_created_workspace_plugins() -> Result<()> {
PluginShareListResponse {
data: vec![PluginShareListItem {
plugin: PluginSummary {
id: "demo-plugin@workspace-shared-with-me-private".to_string(),
id: "demo-plugin@workspace-shared-with-me".to_string(),
remote_plugin_id: Some("plugins_123".to_string()),
local_version: None,
name: "demo-plugin".to_string(),
@@ -1123,7 +1123,7 @@ async fn plugin_share_delete_removes_created_workspace_plugin() -> Result<()> {
PluginShareListResponse {
data: vec![PluginShareListItem {
plugin: PluginSummary {
id: "demo-plugin@workspace-shared-with-me-private".to_string(),
id: "demo-plugin@workspace-shared-with-me".to_string(),
remote_plugin_id: Some("plugins_123".to_string()),
local_version: None,
name: "demo-plugin".to_string(),

View File

@@ -48,6 +48,7 @@ pub use share::update_remote_plugin_share_targets;
pub const REMOTE_GLOBAL_MARKETPLACE_NAME: &str = "chatgpt-global";
pub const REMOTE_WORKSPACE_MARKETPLACE_NAME: &str = "workspace-directory";
pub const REMOTE_WORKSPACE_SHARED_WITH_ME_MARKETPLACE_NAME: &str = "workspace-shared-with-me";
pub const REMOTE_WORKSPACE_SHARED_WITH_ME_PRIVATE_MARKETPLACE_NAME: &str =
"workspace-shared-with-me-private";
pub const REMOTE_WORKSPACE_SHARED_WITH_ME_UNLISTED_MARKETPLACE_NAME: &str =
@@ -296,6 +297,7 @@ impl RemotePluginScope {
match name {
REMOTE_GLOBAL_MARKETPLACE_NAME => Some(Self::Global),
REMOTE_WORKSPACE_MARKETPLACE_NAME
| REMOTE_WORKSPACE_SHARED_WITH_ME_MARKETPLACE_NAME
| REMOTE_WORKSPACE_SHARED_WITH_ME_PRIVATE_MARKETPLACE_NAME
| REMOTE_WORKSPACE_SHARED_WITH_ME_UNLISTED_MARKETPLACE_NAME => Some(Self::Workspace),
_ => None,
@@ -397,11 +399,9 @@ fn remote_plugin_canonical_marketplace_name(
RemotePluginScope::Global => Ok(REMOTE_GLOBAL_MARKETPLACE_NAME),
RemotePluginScope::Workspace => match workspace_plugin_discoverability(plugin)? {
RemotePluginShareDiscoverability::Listed => Ok(REMOTE_WORKSPACE_MARKETPLACE_NAME),
RemotePluginShareDiscoverability::Unlisted => {
Ok(REMOTE_WORKSPACE_SHARED_WITH_ME_UNLISTED_MARKETPLACE_NAME)
}
RemotePluginShareDiscoverability::Private => {
Ok(REMOTE_WORKSPACE_SHARED_WITH_ME_PRIVATE_MARKETPLACE_NAME)
RemotePluginShareDiscoverability::Private
| RemotePluginShareDiscoverability::Unlisted => {
Ok(REMOTE_WORKSPACE_SHARED_WITH_ME_MARKETPLACE_NAME)
}
},
}
@@ -505,20 +505,29 @@ pub async fn fetch_remote_marketplaces(
}
}
RemoteMarketplaceSource::SharedWithMe => {
let private_plugins = fetch_shared_workspace_plugins(config, auth)
.await?
// The shared endpoint is the source of truth for plugins explicitly shared
// with the user. Installed unlisted plugins that are not returned there are
// link-installed and stay in the separate unlisted bucket.
let shared_plugins = fetch_shared_workspace_plugins(config, auth).await?;
let shared_plugin_ids = shared_plugins
.iter()
.map(|plugin| plugin.id.clone())
.collect::<BTreeSet<_>>();
let directly_shared_plugins = shared_plugins
.into_iter()
.filter_map(|plugin| match workspace_plugin_discoverability(&plugin) {
Ok(RemotePluginShareDiscoverability::Private) => Some(Ok(plugin)),
Ok(RemotePluginShareDiscoverability::Listed)
| Ok(RemotePluginShareDiscoverability::Unlisted) => None,
Ok(
RemotePluginShareDiscoverability::Private
| RemotePluginShareDiscoverability::Unlisted,
) => Some(Ok(plugin)),
Ok(RemotePluginShareDiscoverability::Listed) => None,
Err(err) => Some(Err(err)),
})
.collect::<Result<Vec<_>, _>>()?;
if let Some(marketplace) = build_remote_marketplace(
REMOTE_WORKSPACE_SHARED_WITH_ME_PRIVATE_MARKETPLACE_NAME,
REMOTE_WORKSPACE_SHARED_WITH_ME_PRIVATE_MARKETPLACE_DISPLAY_NAME,
private_plugins,
directly_shared_plugins,
workspace_installed_plugins.clone().unwrap_or_default(),
/*include_installed_only*/ false,
)? {
@@ -531,7 +540,12 @@ pub async fn fetch_remote_marketplaces(
.into_iter()
.filter_map(
|plugin| match workspace_plugin_discoverability(&plugin.plugin) {
Ok(RemotePluginShareDiscoverability::Unlisted) => Some(Ok(plugin)),
Ok(RemotePluginShareDiscoverability::Unlisted)
if !shared_plugin_ids.contains(&plugin.plugin.id) =>
{
Some(Ok(plugin))
}
Ok(RemotePluginShareDiscoverability::Unlisted) => None,
Ok(RemotePluginShareDiscoverability::Listed)
| Ok(RemotePluginShareDiscoverability::Private) => None,
Err(err) => Some(Err(err)),

View File

@@ -1,5 +1,6 @@
use super::REMOTE_GLOBAL_MARKETPLACE_NAME;
use super::REMOTE_WORKSPACE_MARKETPLACE_NAME;
use super::REMOTE_WORKSPACE_SHARED_WITH_ME_MARKETPLACE_NAME;
use super::REMOTE_WORKSPACE_SHARED_WITH_ME_PRIVATE_MARKETPLACE_NAME;
use super::REMOTE_WORKSPACE_SHARED_WITH_ME_UNLISTED_MARKETPLACE_NAME;
use super::RemotePluginCatalogError;
@@ -153,6 +154,10 @@ pub async fn sync_remote_installed_plugin_bundles_once(
REMOTE_WORKSPACE_MARKETPLACE_NAME.to_string(),
BTreeSet::new(),
),
(
REMOTE_WORKSPACE_SHARED_WITH_ME_MARKETPLACE_NAME.to_string(),
BTreeSet::new(),
),
(
REMOTE_WORKSPACE_SHARED_WITH_ME_PRIVATE_MARKETPLACE_NAME.to_string(),
BTreeSet::new(),
@@ -303,6 +308,7 @@ fn remove_stale_remote_plugin_caches(
for marketplace_name in [
REMOTE_GLOBAL_MARKETPLACE_NAME,
REMOTE_WORKSPACE_MARKETPLACE_NAME,
REMOTE_WORKSPACE_SHARED_WITH_ME_MARKETPLACE_NAME,
REMOTE_WORKSPACE_SHARED_WITH_ME_PRIVATE_MARKETPLACE_NAME,
REMOTE_WORKSPACE_SHARED_WITH_ME_UNLISTED_MARKETPLACE_NAME,
] {
@@ -510,7 +516,7 @@ mod tests {
}
#[test]
fn stale_remote_plugin_cleanup_removes_private_shared_with_me_cache() {
fn stale_remote_plugin_cleanup_removes_old_shared_with_me_cache_and_keeps_canonical_cache() {
let codex_home = tempfile::tempdir().expect("create codex home");
let cached_manifest = codex_home
.path()
@@ -524,6 +530,18 @@ mod tests {
.expect("create cached plugin manifest parent");
std::fs::write(&cached_manifest, r#"{"name":"private-plugin"}"#)
.expect("write cached plugin manifest");
let canonical_cached_manifest = codex_home
.path()
.join(PLUGINS_CACHE_DIR)
.join(REMOTE_WORKSPACE_SHARED_WITH_ME_MARKETPLACE_NAME)
.join("shared-plugin")
.join("1.2.3")
.join(".codex-plugin")
.join("plugin.json");
std::fs::create_dir_all(canonical_cached_manifest.parent().expect("manifest parent"))
.expect("create canonical cached plugin manifest parent");
std::fs::write(&canonical_cached_manifest, r#"{"name":"shared-plugin"}"#)
.expect("write canonical cached plugin manifest");
let installed_plugin_names_by_marketplace =
BTreeMap::<String, BTreeSet<String>>::from_iter([
(REMOTE_GLOBAL_MARKETPLACE_NAME.to_string(), BTreeSet::new()),
@@ -531,6 +549,10 @@ mod tests {
REMOTE_WORKSPACE_MARKETPLACE_NAME.to_string(),
BTreeSet::new(),
),
(
REMOTE_WORKSPACE_SHARED_WITH_ME_MARKETPLACE_NAME.to_string(),
BTreeSet::from(["shared-plugin".to_string()]),
),
(
REMOTE_WORKSPACE_SHARED_WITH_ME_PRIVATE_MARKETPLACE_NAME.to_string(),
BTreeSet::new(),
@@ -552,5 +574,6 @@ mod tests {
vec!["private-plugin@workspace-shared-with-me-private".to_string()]
);
assert!(!cached_manifest.exists());
assert!(canonical_cached_manifest.is_file());
}
}

View File

@@ -1,3 +1,4 @@
use super::super::REMOTE_WORKSPACE_SHARED_WITH_ME_MARKETPLACE_NAME;
use super::super::REMOTE_WORKSPACE_SHARED_WITH_ME_PRIVATE_MARKETPLACE_NAME;
use super::super::REMOTE_WORKSPACE_SHARED_WITH_ME_UNLISTED_MARKETPLACE_NAME;
use super::super::RemotePluginCatalogError;
@@ -162,7 +163,8 @@ pub async fn checkout_remote_plugin_share(
fn is_checkout_supported_share_marketplace(marketplace_name: &str) -> bool {
matches!(
marketplace_name,
REMOTE_WORKSPACE_SHARED_WITH_ME_PRIVATE_MARKETPLACE_NAME
REMOTE_WORKSPACE_SHARED_WITH_ME_MARKETPLACE_NAME
| REMOTE_WORKSPACE_SHARED_WITH_ME_PRIVATE_MARKETPLACE_NAME
| REMOTE_WORKSPACE_SHARED_WITH_ME_UNLISTED_MARKETPLACE_NAME
)
}

View File

@@ -586,7 +586,7 @@ async fn list_remote_plugin_shares_fetches_created_workspace_plugins() {
vec![
RemotePluginShareSummary {
summary: RemotePluginSummary {
id: "demo-plugin@workspace-shared-with-me-private".to_string(),
id: "demo-plugin@workspace-shared-with-me".to_string(),
remote_plugin_id: "plugins_123".to_string(),
name: "demo-plugin".to_string(),
share_context: Some(RemotePluginShareContext {
@@ -625,7 +625,7 @@ async fn list_remote_plugin_shares_fetches_created_workspace_plugins() {
},
RemotePluginShareSummary {
summary: RemotePluginSummary {
id: "demo-plugin@workspace-shared-with-me-private".to_string(),
id: "demo-plugin@workspace-shared-with-me".to_string(),
remote_plugin_id: "plugins_456".to_string(),
name: "demo-plugin".to_string(),
share_context: Some(RemotePluginShareContext {