diff --git a/codex-rs/app-server/tests/suite/v2/plugin_list.rs b/codex-rs/app-server/tests/suite/v2/plugin_list.rs index 0b2200a9d5..6fccdca9d7 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_list.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_list.rs @@ -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(), diff --git a/codex-rs/app-server/tests/suite/v2/plugin_read.rs b/codex-rs/app-server/tests/suite/v2/plugin_read.rs index 525a4234aa..812f884a55 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_read.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_read.rs @@ -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(()) } diff --git a/codex-rs/app-server/tests/suite/v2/plugin_share.rs b/codex-rs/app-server/tests/suite/v2/plugin_share.rs index efe3d291a4..43f652c1e0 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_share.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_share.rs @@ -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(), diff --git a/codex-rs/core-plugins/src/remote.rs b/codex-rs/core-plugins/src/remote.rs index fc6fedbe23..4dde224039 100644 --- a/codex-rs/core-plugins/src/remote.rs +++ b/codex-rs/core-plugins/src/remote.rs @@ -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::>(); + 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::, _>>()?; 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)), diff --git a/codex-rs/core-plugins/src/remote/remote_installed_plugin_sync.rs b/codex-rs/core-plugins/src/remote/remote_installed_plugin_sync.rs index c94eba428e..b13ca7a630 100644 --- a/codex-rs/core-plugins/src/remote/remote_installed_plugin_sync.rs +++ b/codex-rs/core-plugins/src/remote/remote_installed_plugin_sync.rs @@ -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::>::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()); } } diff --git a/codex-rs/core-plugins/src/remote/share/checkout.rs b/codex-rs/core-plugins/src/remote/share/checkout.rs index 9ccdbee851..8aabccff8b 100644 --- a/codex-rs/core-plugins/src/remote/share/checkout.rs +++ b/codex-rs/core-plugins/src/remote/share/checkout.rs @@ -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 ) } diff --git a/codex-rs/core-plugins/src/remote/share/tests.rs b/codex-rs/core-plugins/src/remote/share/tests.rs index 0b8f8f2fb5..f62051374e 100644 --- a/codex-rs/core-plugins/src/remote/share/tests.rs +++ b/codex-rs/core-plugins/src/remote/share/tests.rs @@ -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 {