mirror of
https://github.com/openai/codex.git
synced 2026-10-28 00:03:58 +00:00
Use batch metadata for plugin app summaries (#34851)
## What changed - Load app metadata for plugin read and install responses through the authenticated batch API, splitting requests into batches of 100 and retaining cached metadata if a batch fails. - Preserve every declared app in plugin responses, using its ID as the fallback name and retaining categories from the plugin declaration when metadata is unavailable. - Add `isEnabled`, `disabledReason`, and `isReadOnly` to `AppToolSummary`, with defaults that keep legacy responses compatible. ## Testing - Cover plugin reads with more than 100 apps, partial batch failures, install-time authentication results, tool-state propagation, and legacy tool summaries. GitOrigin-RevId: 14000df9c5c94dde781358e292a8bef741c5dd23
This commit is contained in:
@@ -89,11 +89,17 @@ pub(crate) fn connector_metadata_to_api(metadata: ConnectorMetadata) -> ApiConne
|
||||
name,
|
||||
title,
|
||||
description,
|
||||
is_enabled,
|
||||
disabled_reason,
|
||||
is_read_only,
|
||||
} = tool;
|
||||
ApiAppToolSummary {
|
||||
name,
|
||||
title,
|
||||
description,
|
||||
is_enabled,
|
||||
disabled_reason,
|
||||
is_read_only,
|
||||
}
|
||||
})
|
||||
.collect()
|
||||
|
||||
@@ -477,7 +477,7 @@ fn record_legacy_apps_installed_duration(started_at: Instant, reload: bool) {
|
||||
);
|
||||
}
|
||||
}
|
||||
const APP_READ_MAX_IDS: usize = 100;
|
||||
pub(super) const APP_READ_MAX_IDS: usize = 100;
|
||||
|
||||
enum AppListLoadResult {
|
||||
Accessible(Result<AccessibleConnectorsStatus, String>),
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
use super::apps_processor::APP_READ_MAX_IDS;
|
||||
use super::*;
|
||||
use crate::error_code::internal_error;
|
||||
use crate::error_code::invalid_request;
|
||||
@@ -1075,6 +1076,7 @@ impl PluginRequestProcessor {
|
||||
};
|
||||
let app_summaries = load_plugin_app_summaries(
|
||||
&config,
|
||||
auth.as_ref(),
|
||||
&outcome.plugin.apps,
|
||||
&outcome.plugin.app_category_by_id,
|
||||
)
|
||||
@@ -1161,8 +1163,13 @@ impl PluginRequestProcessor {
|
||||
.as_ref()
|
||||
.map(plugin_app_category_by_id_from_value)
|
||||
.unwrap_or_default();
|
||||
let app_summaries =
|
||||
load_plugin_app_summaries(&config, &plugin_apps, &app_category_by_id).await;
|
||||
let app_summaries = load_plugin_app_summaries(
|
||||
&config,
|
||||
auth.as_ref(),
|
||||
&plugin_apps,
|
||||
&app_category_by_id,
|
||||
)
|
||||
.await;
|
||||
remote_plugin_detail_to_info(remote_detail, app_summaries)
|
||||
}
|
||||
};
|
||||
@@ -1504,14 +1511,12 @@ impl PluginRequestProcessor {
|
||||
}
|
||||
|
||||
let plugin_app_declarations = load_plugin_apps(result.installed_path.as_path()).await;
|
||||
let plugin_apps =
|
||||
codex_plugin::app_connector_ids_from_declarations(&plugin_app_declarations);
|
||||
let apps_needing_auth = self
|
||||
.plugin_apps_needing_auth_for_install(
|
||||
&config,
|
||||
auth.as_ref().is_some_and(CodexAuth::is_chatgpt_auth),
|
||||
auth.as_ref(),
|
||||
&result.plugin_id.as_key(),
|
||||
&plugin_apps,
|
||||
&plugin_app_declarations,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -1697,35 +1702,16 @@ impl PluginRequestProcessor {
|
||||
.as_ref()
|
||||
.map(plugin_app_category_by_id_from_value)
|
||||
.unwrap_or_default();
|
||||
let all_connectors = connectors::list_cached_all_connectors(&config, &[])
|
||||
load_plugin_app_summaries(&config, auth.as_ref(), &plugin_apps, &app_category_by_id)
|
||||
.await
|
||||
.unwrap_or_default();
|
||||
connectors::connectors_for_plugin_apps(all_connectors, &plugin_apps)
|
||||
.into_iter()
|
||||
.map(|connector| {
|
||||
let category = app_category_by_id
|
||||
.get(&connector.id)
|
||||
.cloned()
|
||||
.or_else(|| connector.category());
|
||||
AppSummary {
|
||||
category,
|
||||
id: connector.id,
|
||||
name: connector.name,
|
||||
description: connector.description,
|
||||
install_url: connector.install_url,
|
||||
}
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
} else {
|
||||
let plugin_app_declarations = load_plugin_apps(result.installed_path.as_path()).await;
|
||||
let plugin_apps =
|
||||
codex_plugin::app_connector_ids_from_declarations(&plugin_app_declarations);
|
||||
self.plugin_apps_needing_auth_for_install(
|
||||
&config,
|
||||
is_chatgpt_auth,
|
||||
auth.as_ref(),
|
||||
&result.plugin_id.as_key(),
|
||||
&plugin_apps,
|
||||
&plugin_app_declarations,
|
||||
)
|
||||
.await
|
||||
};
|
||||
@@ -1775,17 +1761,31 @@ impl PluginRequestProcessor {
|
||||
async fn plugin_apps_needing_auth_for_install(
|
||||
&self,
|
||||
config: &Config,
|
||||
is_chatgpt_auth: bool,
|
||||
auth: Option<&CodexAuth>,
|
||||
plugin_id: &str,
|
||||
plugin_apps: &[codex_plugin::AppConnectorId],
|
||||
plugin_app_declarations: &[codex_plugin::AppDeclaration],
|
||||
) -> Vec<AppSummary> {
|
||||
if plugin_apps.is_empty() || !config.features.apps_enabled_for_auth(is_chatgpt_auth) {
|
||||
if plugin_app_declarations.is_empty()
|
||||
|| !config
|
||||
.features
|
||||
.apps_enabled_for_auth(auth.is_some_and(CodexAuth::is_chatgpt_auth))
|
||||
{
|
||||
return Vec::new();
|
||||
}
|
||||
|
||||
let plugin_apps =
|
||||
codex_plugin::app_connector_ids_from_declarations(plugin_app_declarations);
|
||||
let app_category_by_id = plugin_app_declarations
|
||||
.iter()
|
||||
.filter_map(|app| {
|
||||
app.category
|
||||
.as_ref()
|
||||
.map(|category| (app.connector_id.0.clone(), category.clone()))
|
||||
})
|
||||
.collect();
|
||||
let environment_manager = self.thread_manager.environment_manager();
|
||||
let (all_connectors_result, accessible_connectors_result) = tokio::join!(
|
||||
connectors::list_all_connectors_with_options(config, /*force_refetch*/ false, &[]),
|
||||
let (app_summaries, accessible_connectors_result) = tokio::join!(
|
||||
load_plugin_app_summaries(config, auth, &plugin_apps, &app_category_by_id),
|
||||
connectors::list_accessible_connectors_from_mcp_tools_with_mcp_manager(
|
||||
config,
|
||||
/*force_refetch*/ true,
|
||||
@@ -1794,19 +1794,6 @@ impl PluginRequestProcessor {
|
||||
),
|
||||
);
|
||||
|
||||
let all_connectors = match all_connectors_result {
|
||||
Ok(connectors) => connectors,
|
||||
Err(err) => {
|
||||
warn!(
|
||||
plugin = plugin_id,
|
||||
"failed to load app metadata after plugin install: {err:#}"
|
||||
);
|
||||
connectors::list_cached_all_connectors(config, &[])
|
||||
.await
|
||||
.unwrap_or_default()
|
||||
}
|
||||
};
|
||||
let all_connectors = connectors::connectors_for_plugin_apps(all_connectors, plugin_apps);
|
||||
let (accessible_connectors, codex_apps_ready) = match accessible_connectors_result {
|
||||
Ok(status) => (status.connectors, status.codex_apps_ready),
|
||||
Err(err) => {
|
||||
@@ -1827,14 +1814,17 @@ impl PluginRequestProcessor {
|
||||
plugin = plugin_id,
|
||||
"codex_apps MCP not ready after plugin install; skipping appsNeedingAuth check"
|
||||
);
|
||||
return Vec::new();
|
||||
}
|
||||
|
||||
plugin_apps_needing_auth(
|
||||
&all_connectors,
|
||||
&accessible_connectors,
|
||||
plugin_apps,
|
||||
codex_apps_ready,
|
||||
)
|
||||
let accessible_ids = accessible_connectors
|
||||
.iter()
|
||||
.map(|connector| connector.id.as_str())
|
||||
.collect::<HashSet<_>>();
|
||||
app_summaries
|
||||
.into_iter()
|
||||
.filter(|app| !accessible_ids.contains(app.id.as_str()))
|
||||
.collect()
|
||||
}
|
||||
|
||||
async fn start_plugin_mcp_oauth_logins(
|
||||
@@ -2078,43 +2068,66 @@ impl PluginRequestProcessor {
|
||||
|
||||
async fn load_plugin_app_summaries(
|
||||
config: &Config,
|
||||
auth: Option<&CodexAuth>,
|
||||
plugin_apps: &[codex_plugin::AppConnectorId],
|
||||
app_category_by_id: &HashMap<String, String>,
|
||||
) -> Vec<AppSummary> {
|
||||
if plugin_apps.is_empty() {
|
||||
return Vec::new();
|
||||
let mut seen_app_ids = HashSet::new();
|
||||
let app_ids = plugin_apps
|
||||
.iter()
|
||||
.map(|app| app.0.clone())
|
||||
.filter(|app_id| seen_app_ids.insert(app_id.clone()))
|
||||
.collect::<Vec<_>>();
|
||||
let mut metadata_by_id = HashMap::new();
|
||||
if let Some(auth) = auth.filter(|auth| {
|
||||
config
|
||||
.features
|
||||
.apps_enabled_for_auth(auth.uses_codex_backend())
|
||||
}) {
|
||||
metadata_by_id.extend(
|
||||
codex_connectors::ConnectorMetadataStore::new(
|
||||
config.chatgpt_base_url.clone(),
|
||||
auth.get_account_id(),
|
||||
auth.get_chatgpt_user_id(),
|
||||
auth.is_workspace_account(),
|
||||
)
|
||||
.fresh_records(&app_ids, /*include_tools*/ false),
|
||||
);
|
||||
for app_ids in app_ids.chunks(APP_READ_MAX_IDS) {
|
||||
match connectors::read_connector_metadata(
|
||||
config, auth, app_ids, /*include_tools*/ false,
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(result) => metadata_by_id.extend(
|
||||
result
|
||||
.apps
|
||||
.into_iter()
|
||||
.map(|metadata| (metadata.id.clone(), metadata)),
|
||||
),
|
||||
Err(err) => {
|
||||
warn!("failed to load app metadata for plugin: {err:#}");
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let connectors = match connectors::list_all_connectors_with_options(
|
||||
config,
|
||||
/*force_refetch*/ false,
|
||||
&[],
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(connectors) => connectors,
|
||||
Err(err) => {
|
||||
warn!("failed to load app metadata for plugin/read: {err:#}");
|
||||
connectors::list_cached_all_connectors(config, &[])
|
||||
.await
|
||||
.unwrap_or_default()
|
||||
}
|
||||
};
|
||||
|
||||
let plugin_connectors = connectors::connectors_for_plugin_apps(connectors, plugin_apps);
|
||||
|
||||
plugin_connectors
|
||||
app_ids
|
||||
.into_iter()
|
||||
.map(|connector| {
|
||||
let category = app_category_by_id
|
||||
.get(&connector.id)
|
||||
.cloned()
|
||||
.or_else(|| connector.category());
|
||||
.map(|app_id| {
|
||||
let (name, description) = metadata_by_id
|
||||
.remove(&app_id)
|
||||
.map(|metadata| (metadata.name, metadata.description))
|
||||
.unwrap_or_else(|| (app_id.clone(), None));
|
||||
let category = app_category_by_id.get(&app_id).cloned();
|
||||
AppSummary {
|
||||
id: connector.id,
|
||||
name: connector.name,
|
||||
description: connector.description,
|
||||
install_url: connector.install_url,
|
||||
install_url: Some(codex_connectors::metadata::connector_install_url(
|
||||
&name, &app_id,
|
||||
)),
|
||||
id: app_id,
|
||||
name,
|
||||
description,
|
||||
category,
|
||||
}
|
||||
})
|
||||
@@ -2128,45 +2141,6 @@ fn plugin_app_category_by_id_from_value(value: &serde_json::Value) -> HashMap<St
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn plugin_apps_needing_auth(
|
||||
all_connectors: &[AppInfo],
|
||||
accessible_connectors: &[AppInfo],
|
||||
plugin_apps: &[codex_plugin::AppConnectorId],
|
||||
codex_apps_ready: bool,
|
||||
) -> Vec<AppSummary> {
|
||||
if !codex_apps_ready {
|
||||
return Vec::new();
|
||||
}
|
||||
|
||||
let accessible_ids = accessible_connectors
|
||||
.iter()
|
||||
.map(|connector| connector.id.as_str())
|
||||
.collect::<HashSet<_>>();
|
||||
let plugin_app_ids = plugin_apps
|
||||
.iter()
|
||||
.map(|connector_id| connector_id.0.as_str())
|
||||
.collect::<HashSet<_>>();
|
||||
|
||||
all_connectors
|
||||
.iter()
|
||||
.filter(|connector| {
|
||||
plugin_app_ids.contains(connector.id.as_str())
|
||||
&& !accessible_ids.contains(connector.id.as_str())
|
||||
})
|
||||
.cloned()
|
||||
.map(|connector| {
|
||||
let category = connector.category();
|
||||
AppSummary {
|
||||
category,
|
||||
id: connector.id,
|
||||
name: connector.name,
|
||||
description: connector.description,
|
||||
install_url: connector.install_url,
|
||||
}
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn remote_marketplace_to_info(marketplace: RemoteMarketplace) -> PluginMarketplaceEntry {
|
||||
PluginMarketplaceEntry {
|
||||
name: marketplace.name,
|
||||
|
||||
Reference in New Issue
Block a user