Relax remote plugin sync gate (#22594)

## Summary
- Allow remote installed-plugin cache refresh to start whenever plugins
are enabled.
- Allow remote installed-plugin bundle sync to start whenever plugins
are enabled.
- Remove the extra local `remote_plugin_enabled` guard from those
background sync paths.

## Context
Server-side installed plugin state and optional bundle URL behavior are
owned by plugin-service `/public/plugins/installed`, so these local sync
paths only need the overall plugin enablement gate.

## Test plan
- `just fmt`
- `cargo test -p codex-core-plugins`
This commit is contained in:
xli-oai authored and GitHub committed 2026-05-14 03:38:30 +00:00
1 parent 35451ba79c
commit 9797296564
5 files changed
+88 -36

No files matched your search

@@ -450,14 +450,15 @@ impl PluginRequestProcessor {
return Ok(empty_response()); return Ok(empty_response());
} }
let plugins_input = config.plugins_config_input(); let plugins_input = config.plugins_config_input();
let (mut data, marketplace_load_errors) = if include_local { if include_local || marketplace_kinds.contains(&PluginListMarketplaceKind::SharedWithMe) {
plugins_manager.maybe_start_plugin_list_background_tasks_for_config( plugins_manager.maybe_start_plugin_list_background_tasks_for_config(
&plugins_input, &plugins_input,
auth.clone(), auth.clone(),
&roots, &roots,
Some(self.effective_plugins_changed_callback()), Some(self.effective_plugins_changed_callback()),
); );
}
let (mut data, marketplace_load_errors) = if include_local {
let config_for_marketplace_listing = plugins_input.clone(); let config_for_marketplace_listing = plugins_input.clone();
let plugins_manager_for_marketplace_listing = plugins_manager.clone(); let plugins_manager_for_marketplace_listing = plugins_manager.clone();
let shared_plugin_ids_by_local_path = load_shared_plugin_ids_by_local_path(&config)?; let shared_plugin_ids_by_local_path = load_shared_plugin_ids_by_local_path(&config)?;
@@ -916,6 +917,9 @@ impl PluginRequestProcessor {
params: PluginShareUpdateTargetsParams, params: PluginShareUpdateTargetsParams,
) -> Result<PluginShareUpdateTargetsResponse, JSONRPCErrorError> { ) -> Result<PluginShareUpdateTargetsResponse, JSONRPCErrorError> {
let (config, auth) = self.load_plugin_share_config_and_auth().await?; let (config, auth) = self.load_plugin_share_config_and_auth().await?;
if !config.features.enabled(Feature::PluginSharing) {
return Err(invalid_request("plugin sharing is disabled"));
}
let PluginShareUpdateTargetsParams { let PluginShareUpdateTargetsParams {
remote_plugin_id, remote_plugin_id,
discoverability, discoverability,
@@ -1360,7 +1360,7 @@ async fn app_server_startup_remote_plugin_sync_runs_once() -> Result<()> {
async fn app_server_startup_sync_downloads_remote_installed_plugin_bundles() -> Result<()> { async fn app_server_startup_sync_downloads_remote_installed_plugin_bundles() -> Result<()> {
let codex_home = TempDir::new()?; let codex_home = TempDir::new()?;
let server = MockServer::start().await; let server = MockServer::start().await;
write_remote_plugin_catalog_config( write_plugins_enabled_config_with_base_url(
codex_home.path(), codex_home.path(),
&format!("{}/backend-api/", server.uri()), &format!("{}/backend-api/", server.uri()),
)?; )?;
@@ -1858,6 +1858,7 @@ async fn plugin_list_fetches_shared_with_me_kind() -> Result<()> {
.push(unlisted_installed_body["plugins"][0].clone()); .push(unlisted_installed_body["plugins"][0].clone());
let workspace_installed_body = serde_json::to_string(&workspace_installed_body)?; let workspace_installed_body = serde_json::to_string(&workspace_installed_body)?;
mount_shared_workspace_plugins(&server, &shared_plugin_body).await; mount_shared_workspace_plugins(&server, &shared_plugin_body).await;
mount_remote_installed_plugins(&server, "GLOBAL", empty_remote_installed_plugins_body()).await;
mount_remote_installed_plugins(&server, "WORKSPACE", &workspace_installed_body).await; mount_remote_installed_plugins(&server, "WORKSPACE", &workspace_installed_body).await;
let mut mcp = McpProcess::new(codex_home.path()).await?; let mut mcp = McpProcess::new(codex_home.path()).await?;
@@ -1986,6 +1987,12 @@ async fn plugin_list_fetches_shared_with_me_kind() -> Result<()> {
share_context.discoverability, share_context.discoverability,
Some(PluginShareDiscoverability::Unlisted) Some(PluginShareDiscoverability::Unlisted)
); );
wait_for_remote_plugin_request_count(
&server,
"/ps/plugins/installed",
/*expected_count*/ 5,
)
.await?;
wait_for_remote_plugin_request_count(&server, "/ps/plugins/list", /*expected_count*/ 0).await?; wait_for_remote_plugin_request_count(&server, "/ps/plugins/list", /*expected_count*/ 0).await?;
Ok(()) Ok(())
} }
@@ -1038,6 +1038,57 @@ async fn plugin_share_update_targets_updates_share_targets() -> Result<()> {
Ok(()) Ok(())
} }
#[tokio::test]
async fn plugin_share_update_targets_rejects_when_plugin_sharing_disabled() -> Result<()> {
let codex_home = TempDir::new()?;
let server = MockServer::start().await;
std::fs::write(
codex_home.path().join("config.toml"),
format!(
r#"
chatgpt_base_url = "{}/backend-api"
[features]
plugins = true
remote_plugin = true
plugin_sharing = false
"#,
server.uri()
),
)?;
write_chatgpt_auth(
codex_home.path(),
ChatGptAuthFixture::new("chatgpt-token")
.account_id("account-123")
.chatgpt_user_id("user-123")
.chatgpt_account_id("account-123"),
AuthCredentialsStoreMode::File,
)?;
let mut mcp = McpProcess::new(codex_home.path()).await?;
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
let request_id = mcp
.send_raw_request(
"plugin/share/updateTargets",
Some(json!({
"remotePluginId": "plugins_123",
"discoverability": "UNLISTED",
"shareTargets": [],
})),
)
.await?;
let error: JSONRPCError = timeout(
DEFAULT_TIMEOUT,
mcp.read_stream_until_error_message(RequestId::Integer(request_id)),
)
.await??;
assert_eq!(error.error.code, -32600);
assert_eq!(error.error.message, "plugin sharing is disabled");
Ok(())
}
#[tokio::test] #[tokio::test]
async fn plugin_share_delete_removes_created_workspace_plugin() -> Result<()> { async fn plugin_share_delete_removes_created_workspace_plugin() -> Result<()> {
let codex_home = TempDir::new()?; let codex_home = TempDir::new()?;
+22 -31
View File
@@ -494,7 +494,7 @@ impl PluginsManager {
let outcome = load_plugins_from_layer_stack( let outcome = load_plugins_from_layer_stack(
&config.config_layer_stack, &config.config_layer_stack,
self.remote_installed_plugin_configs(config), self.remote_installed_plugin_configs(),
&self.store, &self.store,
self.restriction_product, self.restriction_product,
plugin_hooks_enabled, plugin_hooks_enabled,
@@ -542,7 +542,7 @@ impl PluginsManager {
} }
load_plugins_from_layer_stack( load_plugins_from_layer_stack(
config_layer_stack, config_layer_stack,
self.remote_installed_plugin_configs(config), self.remote_installed_plugin_configs(),
&self.store, &self.store,
self.restriction_product, self.restriction_product,
plugin_hooks_feature_enabled, plugin_hooks_feature_enabled,
@@ -585,14 +585,7 @@ impl PluginsManager {
} }
} }
fn remote_installed_plugin_configs( fn remote_installed_plugin_configs(&self) -> HashMap<String, PluginConfig> {
&self,
config: &PluginsConfigInput,
) -> HashMap<String, PluginConfig> {
if !config.remote_plugin_enabled {
return HashMap::new();
}
let cache = match self.remote_installed_plugins_cache.read() { let cache = match self.remote_installed_plugins_cache.read() {
Ok(cache) => cache, Ok(cache) => cache,
Err(err) => err.into_inner(), Err(err) => err.into_inner(),
@@ -667,7 +660,7 @@ impl PluginsManager {
notify: RemoteInstalledPluginsCacheRefreshNotify, notify: RemoteInstalledPluginsCacheRefreshNotify,
on_effective_plugins_changed: Option<Arc<dyn Fn() + Send + Sync + 'static>>, on_effective_plugins_changed: Option<Arc<dyn Fn() + Send + Sync + 'static>>,
) { ) {
if !config.plugins_enabled || !config.remote_plugin_enabled { if !config.plugins_enabled {
return; return;
} }
@@ -687,7 +680,7 @@ impl PluginsManager {
auth: Option<CodexAuth>, auth: Option<CodexAuth>,
on_effective_plugins_changed: Option<Arc<dyn Fn() + Send + Sync + 'static>>, on_effective_plugins_changed: Option<Arc<dyn Fn() + Send + Sync + 'static>>,
) { ) {
if !config.plugins_enabled || !config.remote_plugin_enabled { if !config.plugins_enabled {
return; return;
} }
@@ -1504,25 +1497,23 @@ impl PluginsManager {
auth_manager.clone(), auth_manager.clone(),
); );
if config.remote_plugin_enabled { let config_for_remote_sync = config.clone();
let config = config.clone(); let manager = Arc::clone(self);
let manager = Arc::clone(self); let auth_manager_for_remote_sync = auth_manager.clone();
let auth_manager = auth_manager.clone(); let on_effective_plugins_changed = on_effective_plugins_changed.clone();
let on_effective_plugins_changed = on_effective_plugins_changed.clone(); tokio::spawn(async move {
tokio::spawn(async move { let auth = auth_manager_for_remote_sync.auth().await;
let auth = auth_manager.auth().await; manager.maybe_start_remote_installed_plugins_cache_refresh(
manager.maybe_start_remote_installed_plugins_cache_refresh( &config_for_remote_sync,
&config, auth.clone(),
auth.clone(), on_effective_plugins_changed.clone(),
on_effective_plugins_changed.clone(), );
); manager.maybe_start_remote_installed_plugin_bundle_sync(
manager.maybe_start_remote_installed_plugin_bundle_sync( &config_for_remote_sync,
&config, auth,
auth, on_effective_plugins_changed,
on_effective_plugins_changed, );
); });
});
}
let config = config.clone(); let config = config.clone();
let manager = Arc::clone(self); let manager = Arc::clone(self);
+1 -2
View File
@@ -331,7 +331,7 @@ approval_mode = "approve"
} }
#[tokio::test] #[tokio::test]
async fn remote_installed_cache_adds_plugin_skill_roots_without_marketplace_config() { async fn remote_installed_cache_adds_plugin_skill_roots_without_remote_plugin_flag() {
let codex_home = TempDir::new().unwrap(); let codex_home = TempDir::new().unwrap();
let plugin_base = codex_home let plugin_base = codex_home
.path() .path()
@@ -341,7 +341,6 @@ async fn remote_installed_cache_adds_plugin_skill_roots_without_marketplace_conf
&codex_home.path().join(CONFIG_TOML_FILE), &codex_home.path().join(CONFIG_TOML_FILE),
r#"[features] r#"[features]
plugins = true plugins = true
remote_plugin = true
"#, "#,
); );