mirror of
https://github.com/openai/codex.git
synced 2026-11-01 19:33:35 +00:00
Allow plugin-declared first-party connectors
This commit is contained in:
Generated
+1
-1
@@ -2448,6 +2448,7 @@ dependencies = [
|
||||
"async-channel",
|
||||
"codex-async-utils",
|
||||
"codex-config",
|
||||
"codex-connectors",
|
||||
"codex-exec-server",
|
||||
"codex-login",
|
||||
"codex-otel",
|
||||
@@ -3225,7 +3226,6 @@ name = "codex-utils-plugins"
|
||||
version = "0.0.0"
|
||||
dependencies = [
|
||||
"codex-exec-server",
|
||||
"codex-login",
|
||||
"codex-utils-absolute-path",
|
||||
"serde",
|
||||
"serde_json",
|
||||
|
||||
@@ -6268,6 +6268,8 @@ impl CodexMessageProcessor {
|
||||
connectors::list_cached_all_connectors(&config)
|
||||
);
|
||||
let cached_all_connectors = all_connectors.clone();
|
||||
let plugin_declared_connector_ids =
|
||||
connectors::plugin_declared_connector_ids_for_config(&config).await;
|
||||
|
||||
let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel();
|
||||
|
||||
@@ -6301,6 +6303,7 @@ impl CodexMessageProcessor {
|
||||
apps_list_helpers::merge_loaded_apps(
|
||||
all_connectors.as_deref(),
|
||||
accessible_connectors.as_deref(),
|
||||
&plugin_declared_connector_ids,
|
||||
),
|
||||
&config,
|
||||
);
|
||||
@@ -6387,6 +6390,7 @@ impl CodexMessageProcessor {
|
||||
apps_list_helpers::merge_loaded_apps(
|
||||
all_connectors_for_update,
|
||||
accessible_connectors_for_update,
|
||||
&plugin_declared_connector_ids,
|
||||
),
|
||||
&config,
|
||||
);
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
use std::collections::HashSet;
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_app_server_protocol::AppInfo;
|
||||
@@ -13,11 +14,17 @@ use crate::outgoing_message::OutgoingMessageSender;
|
||||
pub(super) fn merge_loaded_apps(
|
||||
all_connectors: Option<&[AppInfo]>,
|
||||
accessible_connectors: Option<&[AppInfo]>,
|
||||
plugin_declared_connector_ids: &HashSet<String>,
|
||||
) -> Vec<AppInfo> {
|
||||
let all_connectors_loaded = all_connectors.is_some();
|
||||
let all = all_connectors.map_or_else(Vec::new, <[AppInfo]>::to_vec);
|
||||
let accessible = accessible_connectors.map_or_else(Vec::new, <[AppInfo]>::to_vec);
|
||||
connectors::merge_connectors_with_accessible(all, accessible, all_connectors_loaded)
|
||||
connectors::merge_connectors_with_accessible(
|
||||
all,
|
||||
accessible,
|
||||
all_connectors_loaded,
|
||||
plugin_declared_connector_ids,
|
||||
)
|
||||
}
|
||||
|
||||
pub(super) fn should_send_app_list_updated_notification(
|
||||
|
||||
@@ -1066,12 +1066,15 @@ impl MessageProcessor {
|
||||
return;
|
||||
}
|
||||
};
|
||||
let plugin_declared_connector_ids =
|
||||
connectors::plugin_declared_connector_ids_for_config(&config).await;
|
||||
|
||||
let data = connectors::with_app_enabled_state(
|
||||
connectors::merge_connectors_with_accessible(
|
||||
all_connectors,
|
||||
accessible_connectors,
|
||||
/*all_connectors_loaded*/ true,
|
||||
&plugin_declared_connector_ids,
|
||||
),
|
||||
&config,
|
||||
);
|
||||
|
||||
@@ -47,9 +47,13 @@ pub async fn list_connectors(config: &Config) -> anyhow::Result<Vec<AppInfo>> {
|
||||
);
|
||||
let connectors = connectors_result?;
|
||||
let accessible = accessible_result?;
|
||||
let plugin_declared_connector_ids = plugin_declared_connector_ids_for_config(config).await;
|
||||
Ok(with_app_enabled_state(
|
||||
merge_connectors_with_accessible(
|
||||
connectors, accessible, /*all_connectors_loaded*/ true,
|
||||
connectors,
|
||||
accessible,
|
||||
/*all_connectors_loaded*/ true,
|
||||
&plugin_declared_connector_ids,
|
||||
),
|
||||
config,
|
||||
))
|
||||
@@ -73,16 +77,16 @@ pub async fn list_cached_all_connectors(config: &Config) -> Option<Vec<AppInfo>>
|
||||
let token_data = get_chatgpt_token_data()?;
|
||||
let cache_key = all_connectors_cache_key(config, &token_data);
|
||||
let connectors = codex_connectors::cached_all_connectors(&cache_key)?;
|
||||
let plugin_apps = plugin_apps_for_config(config).await;
|
||||
let plugin_declared_connector_ids = plugin_declared_connector_ids(&plugin_apps);
|
||||
let connectors = merge_plugin_connectors(
|
||||
connectors,
|
||||
plugin_apps_for_config(config)
|
||||
.await
|
||||
.into_iter()
|
||||
.map(|connector_id| connector_id.0),
|
||||
plugin_apps.into_iter().map(|connector_id| connector_id.0),
|
||||
);
|
||||
Some(filter_disallowed_connectors(
|
||||
connectors,
|
||||
originator().value.as_str(),
|
||||
&plugin_declared_connector_ids,
|
||||
))
|
||||
}
|
||||
|
||||
@@ -113,16 +117,16 @@ pub async fn list_all_connectors_with_options(
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
let plugin_apps = plugin_apps_for_config(config).await;
|
||||
let plugin_declared_connector_ids = plugin_declared_connector_ids(&plugin_apps);
|
||||
let connectors = merge_plugin_connectors(
|
||||
connectors,
|
||||
plugin_apps_for_config(config)
|
||||
.await
|
||||
.into_iter()
|
||||
.map(|connector_id| connector_id.0),
|
||||
plugin_apps.into_iter().map(|connector_id| connector_id.0),
|
||||
);
|
||||
Ok(filter_disallowed_connectors(
|
||||
connectors,
|
||||
originator().value.as_str(),
|
||||
&plugin_declared_connector_ids,
|
||||
))
|
||||
}
|
||||
|
||||
@@ -142,6 +146,17 @@ async fn plugin_apps_for_config(config: &Config) -> Vec<codex_core::plugins::App
|
||||
.effective_apps()
|
||||
}
|
||||
|
||||
pub async fn plugin_declared_connector_ids_for_config(config: &Config) -> HashSet<String> {
|
||||
plugin_declared_connector_ids(&plugin_apps_for_config(config).await)
|
||||
}
|
||||
|
||||
fn plugin_declared_connector_ids(plugin_apps: &[AppConnectorId]) -> HashSet<String> {
|
||||
plugin_apps
|
||||
.iter()
|
||||
.map(|connector_id| connector_id.0.clone())
|
||||
.collect()
|
||||
}
|
||||
|
||||
pub fn connectors_for_plugin_apps(
|
||||
connectors: Vec<AppInfo>,
|
||||
plugin_apps: &[AppConnectorId],
|
||||
@@ -150,6 +165,7 @@ pub fn connectors_for_plugin_apps(
|
||||
.iter()
|
||||
.map(|connector_id| connector_id.0.as_str())
|
||||
.collect::<HashSet<_>>();
|
||||
let plugin_declared_connector_ids = plugin_declared_connector_ids(plugin_apps);
|
||||
|
||||
let connectors = merge_plugin_connectors(
|
||||
connectors,
|
||||
@@ -157,16 +173,21 @@ pub fn connectors_for_plugin_apps(
|
||||
.iter()
|
||||
.map(|connector_id| connector_id.0.clone()),
|
||||
);
|
||||
filter_disallowed_connectors(connectors, originator().value.as_str())
|
||||
.into_iter()
|
||||
.filter(|connector| plugin_app_ids.contains(connector.id.as_str()))
|
||||
.collect()
|
||||
filter_disallowed_connectors(
|
||||
connectors,
|
||||
originator().value.as_str(),
|
||||
&plugin_declared_connector_ids,
|
||||
)
|
||||
.into_iter()
|
||||
.filter(|connector| plugin_app_ids.contains(connector.id.as_str()))
|
||||
.collect()
|
||||
}
|
||||
|
||||
pub fn merge_connectors_with_accessible(
|
||||
connectors: Vec<AppInfo>,
|
||||
accessible_connectors: Vec<AppInfo>,
|
||||
all_connectors_loaded: bool,
|
||||
plugin_declared_connector_ids: &HashSet<String>,
|
||||
) -> Vec<AppInfo> {
|
||||
let accessible_connectors = if all_connectors_loaded {
|
||||
let connector_ids: HashSet<&str> = connectors
|
||||
@@ -181,7 +202,11 @@ pub fn merge_connectors_with_accessible(
|
||||
accessible_connectors
|
||||
};
|
||||
let merged = merge_connectors(connectors, accessible_connectors);
|
||||
filter_disallowed_connectors(merged, originator().value.as_str())
|
||||
filter_disallowed_connectors(
|
||||
merged,
|
||||
originator().value.as_str(),
|
||||
plugin_declared_connector_ids,
|
||||
)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
@@ -233,6 +258,7 @@ mod tests {
|
||||
vec![app("alpha")],
|
||||
vec![app("alpha"), app("beta")],
|
||||
/*all_connectors_loaded*/ true,
|
||||
&HashSet::new(),
|
||||
);
|
||||
assert_eq!(merged, vec![merged_app("alpha", /*is_accessible*/ true)]);
|
||||
}
|
||||
@@ -243,6 +269,7 @@ mod tests {
|
||||
vec![app("alpha")],
|
||||
vec![app("alpha"), app("beta")],
|
||||
/*all_connectors_loaded*/ false,
|
||||
&HashSet::new(),
|
||||
);
|
||||
assert_eq!(
|
||||
merged,
|
||||
@@ -253,6 +280,31 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filters_disallowed_accessible_connectors_while_all_loading() {
|
||||
let plugin_declared_connector_ids =
|
||||
HashSet::from(["connector_openai_appgarden".to_string()]);
|
||||
let merged = merge_connectors_with_accessible(
|
||||
Vec::new(),
|
||||
vec![
|
||||
app("connector_openai_appgarden"),
|
||||
app("connector_openai_hidden"),
|
||||
app("asdk_app_6938a94a61d881918ef32cb999ff937c"),
|
||||
app("beta"),
|
||||
],
|
||||
/*all_connectors_loaded*/ false,
|
||||
&plugin_declared_connector_ids,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
merged,
|
||||
vec![
|
||||
merged_app("beta", /*is_accessible*/ true),
|
||||
merged_app("connector_openai_appgarden", /*is_accessible*/ true),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn connectors_for_plugin_apps_returns_only_requested_plugin_apps() {
|
||||
let connectors = connectors_for_plugin_apps(
|
||||
@@ -278,4 +330,20 @@ mod tests {
|
||||
);
|
||||
assert_eq!(connectors, Vec::<AppInfo>::new());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn connectors_for_plugin_apps_returns_openai_prefixed_plugin_apps() {
|
||||
let connectors = connectors_for_plugin_apps(
|
||||
Vec::new(),
|
||||
&[AppConnectorId("connector_openai_appgarden".to_string())],
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
connectors,
|
||||
vec![merged_app(
|
||||
"connector_openai_appgarden",
|
||||
/*is_accessible*/ false
|
||||
)]
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -16,6 +16,7 @@ anyhow = { workspace = true }
|
||||
async-channel = { workspace = true }
|
||||
codex-async-utils = { workspace = true }
|
||||
codex-config = { workspace = true }
|
||||
codex-connectors = { workspace = true }
|
||||
codex-exec-server = { workspace = true }
|
||||
codex-login = { workspace = true }
|
||||
codex-otel = { workspace = true }
|
||||
|
||||
@@ -14,6 +14,7 @@ pub use skill_dependencies::canonical_mcp_server_key;
|
||||
pub use skill_dependencies::collect_missing_mcp_dependencies;
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
use std::env;
|
||||
use std::path::PathBuf;
|
||||
use std::time::Duration;
|
||||
@@ -152,6 +153,19 @@ impl ToolPluginProvenance {
|
||||
.unwrap_or(&[])
|
||||
}
|
||||
|
||||
pub fn connector_id_is_plugin_declared(&self, connector_id: &str) -> bool {
|
||||
!self
|
||||
.plugin_display_names_for_connector_id(connector_id)
|
||||
.is_empty()
|
||||
}
|
||||
|
||||
pub fn plugin_declared_connector_ids(&self) -> HashSet<String> {
|
||||
self.plugin_display_names_by_connector_id
|
||||
.keys()
|
||||
.cloned()
|
||||
.collect()
|
||||
}
|
||||
|
||||
pub fn plugin_display_names_for_mcp_server_name(&self, server_name: &str) -> &[String] {
|
||||
self.plugin_display_names_by_mcp_server_name
|
||||
.get(server_name)
|
||||
@@ -159,7 +173,9 @@ impl ToolPluginProvenance {
|
||||
.unwrap_or(&[])
|
||||
}
|
||||
|
||||
fn from_capability_summaries(capability_summaries: &[PluginCapabilitySummary]) -> Self {
|
||||
pub(crate) fn from_capability_summaries(
|
||||
capability_summaries: &[PluginCapabilitySummary],
|
||||
) -> Self {
|
||||
let mut tool_plugin_provenance = Self::default();
|
||||
for plugin in capability_summaries {
|
||||
for connector_id in &plugin.app_connector_ids {
|
||||
|
||||
@@ -6,6 +6,7 @@ use codex_plugin::PluginCapabilitySummary;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
use std::path::PathBuf;
|
||||
|
||||
fn test_mcp_config(codex_home: PathBuf) -> McpConfig {
|
||||
@@ -123,6 +124,16 @@ fn tool_plugin_provenance_collects_app_and_mcp_sources() {
|
||||
]),
|
||||
}
|
||||
);
|
||||
assert!(provenance.connector_id_is_plugin_declared("connector_example"));
|
||||
assert!(provenance.connector_id_is_plugin_declared("connector_gmail"));
|
||||
assert!(!provenance.connector_id_is_plugin_declared("connector_missing"));
|
||||
assert_eq!(
|
||||
provenance.plugin_declared_connector_ids(),
|
||||
HashSet::from([
|
||||
"connector_example".to_string(),
|
||||
"connector_gmail".to_string()
|
||||
])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -93,8 +93,9 @@ use url::Url;
|
||||
|
||||
use codex_config::McpServerConfig;
|
||||
use codex_config::McpServerTransportConfig;
|
||||
use codex_connectors::filter::connector_id_passes_disallow_filter;
|
||||
use codex_login::CodexAuth;
|
||||
use codex_utils_plugins::mcp_connector::is_connector_id_allowed;
|
||||
use codex_login::default_client::originator;
|
||||
use codex_utils_plugins::mcp_connector::sanitize_name;
|
||||
|
||||
/// Delimiter used to separate MCP tool-name parts.
|
||||
@@ -448,11 +449,11 @@ struct ManagedClient {
|
||||
}
|
||||
|
||||
impl ManagedClient {
|
||||
fn listed_tools(&self) -> Vec<ToolInfo> {
|
||||
fn listed_tools(&self, tool_plugin_provenance: &ToolPluginProvenance) -> Vec<ToolInfo> {
|
||||
let total_start = Instant::now();
|
||||
if let Some(cache_context) = self.codex_apps_tools_cache_context.as_ref()
|
||||
&& let CachedCodexAppsToolsLoad::Hit(tools) =
|
||||
load_cached_codex_apps_tools(cache_context)
|
||||
load_cached_codex_apps_tools(cache_context, tool_plugin_provenance)
|
||||
{
|
||||
emit_duration(
|
||||
MCP_TOOLS_LIST_DURATION_METRIC,
|
||||
@@ -501,9 +502,11 @@ impl AsyncManagedClient {
|
||||
let startup_snapshot = load_startup_cached_codex_apps_tools_snapshot(
|
||||
&server_name,
|
||||
codex_apps_tools_cache_context.as_ref(),
|
||||
tool_plugin_provenance.as_ref(),
|
||||
)
|
||||
.map(|tools| filter_tools(tools, &tool_filter));
|
||||
let startup_tool_filter = tool_filter;
|
||||
let startup_tool_plugin_provenance = Arc::clone(&tool_plugin_provenance);
|
||||
let startup_complete = Arc::new(AtomicBool::new(false));
|
||||
let startup_complete_for_fut = Arc::clone(&startup_complete);
|
||||
let fut = async move {
|
||||
@@ -533,6 +536,7 @@ impl AsyncManagedClient {
|
||||
tx_event,
|
||||
elicitation_requests,
|
||||
codex_apps_tools_cache_context,
|
||||
tool_plugin_provenance: startup_tool_plugin_provenance,
|
||||
},
|
||||
)
|
||||
.or_cancel(&cancel_token)
|
||||
@@ -631,7 +635,7 @@ impl AsyncManagedClient {
|
||||
Some(startup_tools)
|
||||
} else {
|
||||
match self.client().await {
|
||||
Ok(client) => Some(client.listed_tools()),
|
||||
Ok(client) => Some(client.listed_tools(self.tool_plugin_provenance.as_ref())),
|
||||
Err(_) => self.startup_snapshot.clone(),
|
||||
}
|
||||
};
|
||||
@@ -939,10 +943,12 @@ impl McpConnectionManager {
|
||||
/// latest filtered tool map is returned directly to the caller. On
|
||||
/// failure, the existing cache remains unchanged.
|
||||
pub async fn hard_refresh_codex_apps_tools_cache(&self) -> Result<HashMap<String, ToolInfo>> {
|
||||
let managed_client = self
|
||||
let async_managed_client = self
|
||||
.clients
|
||||
.get(CODEX_APPS_MCP_SERVER_NAME)
|
||||
.ok_or_else(|| anyhow!("unknown MCP server '{CODEX_APPS_MCP_SERVER_NAME}'"))?
|
||||
.ok_or_else(|| anyhow!("unknown MCP server '{CODEX_APPS_MCP_SERVER_NAME}'"))?;
|
||||
let tool_plugin_provenance = Arc::clone(&async_managed_client.tool_plugin_provenance);
|
||||
let managed_client = async_managed_client
|
||||
.client()
|
||||
.await
|
||||
.context("failed to get client")?;
|
||||
@@ -954,6 +960,7 @@ impl McpConnectionManager {
|
||||
&managed_client.client,
|
||||
managed_client.tool_timeout,
|
||||
managed_client.server_instructions.as_deref(),
|
||||
tool_plugin_provenance.as_ref(),
|
||||
)
|
||||
.await
|
||||
.with_context(|| {
|
||||
@@ -1438,6 +1445,7 @@ async fn start_server_task(
|
||||
tx_event,
|
||||
elicitation_requests,
|
||||
codex_apps_tools_cache_context,
|
||||
tool_plugin_provenance,
|
||||
} = params;
|
||||
let elicitation = elicitation_capability_for_server(&server_name);
|
||||
let params = InitializeRequestParams {
|
||||
@@ -1481,6 +1489,7 @@ async fn start_server_task(
|
||||
&client,
|
||||
startup_timeout,
|
||||
initialize_result.instructions.as_deref(),
|
||||
tool_plugin_provenance.as_ref(),
|
||||
)
|
||||
.await
|
||||
.map_err(StartupOutcomeError::from)?;
|
||||
@@ -1523,6 +1532,7 @@ struct StartServerTaskParams {
|
||||
tx_event: Sender<Event>,
|
||||
elicitation_requests: ElicitationRequestManager,
|
||||
codex_apps_tools_cache_context: Option<CodexAppsToolsCacheContext>,
|
||||
tool_plugin_provenance: Arc<ToolPluginProvenance>,
|
||||
}
|
||||
|
||||
async fn make_rmcp_client(
|
||||
@@ -1648,6 +1658,7 @@ fn write_cached_codex_apps_tools_if_needed(
|
||||
fn load_startup_cached_codex_apps_tools_snapshot(
|
||||
server_name: &str,
|
||||
cache_context: Option<&CodexAppsToolsCacheContext>,
|
||||
tool_plugin_provenance: &ToolPluginProvenance,
|
||||
) -> Option<Vec<ToolInfo>> {
|
||||
if server_name != CODEX_APPS_MCP_SERVER_NAME {
|
||||
return None;
|
||||
@@ -1655,7 +1666,7 @@ fn load_startup_cached_codex_apps_tools_snapshot(
|
||||
|
||||
let cache_context = cache_context?;
|
||||
|
||||
match load_cached_codex_apps_tools(cache_context) {
|
||||
match load_cached_codex_apps_tools(cache_context, tool_plugin_provenance) {
|
||||
CachedCodexAppsToolsLoad::Hit(tools) => Some(tools),
|
||||
CachedCodexAppsToolsLoad::Missing | CachedCodexAppsToolsLoad::Invalid => None,
|
||||
}
|
||||
@@ -1664,8 +1675,9 @@ fn load_startup_cached_codex_apps_tools_snapshot(
|
||||
#[cfg(test)]
|
||||
fn read_cached_codex_apps_tools(
|
||||
cache_context: &CodexAppsToolsCacheContext,
|
||||
tool_plugin_provenance: &ToolPluginProvenance,
|
||||
) -> Option<Vec<ToolInfo>> {
|
||||
match load_cached_codex_apps_tools(cache_context) {
|
||||
match load_cached_codex_apps_tools(cache_context, tool_plugin_provenance) {
|
||||
CachedCodexAppsToolsLoad::Hit(tools) => Some(tools),
|
||||
CachedCodexAppsToolsLoad::Missing | CachedCodexAppsToolsLoad::Invalid => None,
|
||||
}
|
||||
@@ -1673,6 +1685,19 @@ fn read_cached_codex_apps_tools(
|
||||
|
||||
fn load_cached_codex_apps_tools(
|
||||
cache_context: &CodexAppsToolsCacheContext,
|
||||
tool_plugin_provenance: &ToolPluginProvenance,
|
||||
) -> CachedCodexAppsToolsLoad {
|
||||
match load_raw_cached_codex_apps_tools(cache_context) {
|
||||
CachedCodexAppsToolsLoad::Hit(tools) => CachedCodexAppsToolsLoad::Hit(
|
||||
filter_disallowed_codex_apps_tools(tools, tool_plugin_provenance),
|
||||
),
|
||||
CachedCodexAppsToolsLoad::Missing => CachedCodexAppsToolsLoad::Missing,
|
||||
CachedCodexAppsToolsLoad::Invalid => CachedCodexAppsToolsLoad::Invalid,
|
||||
}
|
||||
}
|
||||
|
||||
fn load_raw_cached_codex_apps_tools(
|
||||
cache_context: &CodexAppsToolsCacheContext,
|
||||
) -> CachedCodexAppsToolsLoad {
|
||||
let cache_path = cache_context.cache_path();
|
||||
let bytes = match std::fs::read(cache_path) {
|
||||
@@ -1689,7 +1714,7 @@ fn load_cached_codex_apps_tools(
|
||||
if cache.schema_version != CODEX_APPS_TOOLS_CACHE_SCHEMA_VERSION {
|
||||
return CachedCodexAppsToolsLoad::Invalid;
|
||||
}
|
||||
CachedCodexAppsToolsLoad::Hit(filter_disallowed_codex_apps_tools(cache.tools))
|
||||
CachedCodexAppsToolsLoad::Hit(cache.tools)
|
||||
}
|
||||
|
||||
fn write_cached_codex_apps_tools(cache_context: &CodexAppsToolsCacheContext, tools: &[ToolInfo]) {
|
||||
@@ -1699,23 +1724,43 @@ fn write_cached_codex_apps_tools(cache_context: &CodexAppsToolsCacheContext, too
|
||||
{
|
||||
return;
|
||||
}
|
||||
let tools = filter_disallowed_codex_apps_tools(tools.to_vec());
|
||||
let Ok(bytes) = serde_json::to_vec_pretty(&CodexAppsToolsDiskCache {
|
||||
schema_version: CODEX_APPS_TOOLS_CACHE_SCHEMA_VERSION,
|
||||
tools,
|
||||
tools: tools.to_vec(),
|
||||
}) else {
|
||||
return;
|
||||
};
|
||||
let _ = std::fs::write(cache_path, bytes);
|
||||
}
|
||||
|
||||
fn filter_disallowed_codex_apps_tools(tools: Vec<ToolInfo>) -> Vec<ToolInfo> {
|
||||
fn filter_disallowed_codex_apps_tools_for_server(
|
||||
server_name: &str,
|
||||
tools: Vec<ToolInfo>,
|
||||
tool_plugin_provenance: &ToolPluginProvenance,
|
||||
) -> Vec<ToolInfo> {
|
||||
if server_name == CODEX_APPS_MCP_SERVER_NAME {
|
||||
filter_disallowed_codex_apps_tools(tools, tool_plugin_provenance)
|
||||
} else {
|
||||
tools
|
||||
}
|
||||
}
|
||||
|
||||
fn filter_disallowed_codex_apps_tools(
|
||||
tools: Vec<ToolInfo>,
|
||||
tool_plugin_provenance: &ToolPluginProvenance,
|
||||
) -> Vec<ToolInfo> {
|
||||
let plugin_declared_connector_ids = tool_plugin_provenance.plugin_declared_connector_ids();
|
||||
let originator_value = originator();
|
||||
tools
|
||||
.into_iter()
|
||||
.filter(|tool| {
|
||||
tool.connector_id
|
||||
.as_deref()
|
||||
.is_none_or(is_connector_id_allowed)
|
||||
tool.connector_id.as_deref().is_none_or(|connector_id| {
|
||||
connector_id_passes_disallow_filter(
|
||||
connector_id,
|
||||
originator_value.value.as_str(),
|
||||
&plugin_declared_connector_ids,
|
||||
)
|
||||
})
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
@@ -1741,6 +1786,7 @@ async fn list_tools_for_client_uncached(
|
||||
client: &Arc<RmcpClient>,
|
||||
timeout: Option<Duration>,
|
||||
server_instructions: Option<&str>,
|
||||
tool_plugin_provenance: &ToolPluginProvenance,
|
||||
) -> Result<Vec<ToolInfo>> {
|
||||
let resp = client
|
||||
.list_tools_with_connector_ids(/*params*/ None, timeout)
|
||||
@@ -1782,10 +1828,11 @@ async fn list_tools_for_client_uncached(
|
||||
}
|
||||
})
|
||||
.collect();
|
||||
if server_name == CODEX_APPS_MCP_SERVER_NAME {
|
||||
return Ok(filter_disallowed_codex_apps_tools(tools));
|
||||
}
|
||||
Ok(tools)
|
||||
Ok(filter_disallowed_codex_apps_tools_for_server(
|
||||
server_name,
|
||||
tools,
|
||||
tool_plugin_provenance,
|
||||
))
|
||||
}
|
||||
|
||||
fn validate_mcp_server_name(server_name: &str) -> Result<()> {
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
use super::*;
|
||||
use codex_plugin::AppConnectorId;
|
||||
use codex_plugin::PluginCapabilitySummary;
|
||||
use codex_protocol::ToolName;
|
||||
use codex_protocol::protocol::GranularApprovalConfig;
|
||||
use codex_protocol::protocol::McpAuthStatus;
|
||||
@@ -62,6 +64,14 @@ fn create_codex_apps_tools_cache_context(
|
||||
}
|
||||
}
|
||||
|
||||
fn provenance_for_connector(connector_id: &str) -> ToolPluginProvenance {
|
||||
ToolPluginProvenance::from_capability_summaries(&[PluginCapabilitySummary {
|
||||
display_name: "sample-plugin".to_string(),
|
||||
app_connector_ids: vec![AppConnectorId(connector_id.to_string())],
|
||||
..PluginCapabilitySummary::default()
|
||||
}])
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn declared_openai_file_fields_treat_names_literally() {
|
||||
let meta = serde_json::json!({
|
||||
@@ -480,12 +490,14 @@ fn codex_apps_tools_cache_is_overwritten_by_last_write() {
|
||||
|
||||
write_cached_codex_apps_tools(&cache_context, &tools_gateway_1);
|
||||
let cached_gateway_1 =
|
||||
read_cached_codex_apps_tools(&cache_context).expect("cache entry exists for first write");
|
||||
read_cached_codex_apps_tools(&cache_context, &ToolPluginProvenance::default())
|
||||
.expect("cache entry exists for first write");
|
||||
assert_eq!(cached_gateway_1[0].callable_name, "one");
|
||||
|
||||
write_cached_codex_apps_tools(&cache_context, &tools_gateway_2);
|
||||
let cached_gateway_2 =
|
||||
read_cached_codex_apps_tools(&cache_context).expect("cache entry exists for second write");
|
||||
read_cached_codex_apps_tools(&cache_context, &ToolPluginProvenance::default())
|
||||
.expect("cache entry exists for second write");
|
||||
assert_eq!(cached_gateway_2[0].callable_name, "two");
|
||||
}
|
||||
|
||||
@@ -509,9 +521,11 @@ fn codex_apps_tools_cache_is_scoped_per_user() {
|
||||
write_cached_codex_apps_tools(&cache_context_user_2, &tools_user_2);
|
||||
|
||||
let read_user_1 =
|
||||
read_cached_codex_apps_tools(&cache_context_user_1).expect("cache entry for user one");
|
||||
read_cached_codex_apps_tools(&cache_context_user_1, &ToolPluginProvenance::default())
|
||||
.expect("cache entry for user one");
|
||||
let read_user_2 =
|
||||
read_cached_codex_apps_tools(&cache_context_user_2).expect("cache entry for user two");
|
||||
read_cached_codex_apps_tools(&cache_context_user_2, &ToolPluginProvenance::default())
|
||||
.expect("cache entry for user two");
|
||||
|
||||
assert_eq!(read_user_1[0].callable_name, "one");
|
||||
assert_eq!(read_user_2[0].callable_name, "two");
|
||||
@@ -546,13 +560,74 @@ fn codex_apps_tools_cache_filters_disallowed_connectors() {
|
||||
];
|
||||
|
||||
write_cached_codex_apps_tools(&cache_context, &tools);
|
||||
let cached = read_cached_codex_apps_tools(&cache_context).expect("cache entry exists for user");
|
||||
let cached = read_cached_codex_apps_tools(&cache_context, &ToolPluginProvenance::default())
|
||||
.expect("cache entry exists for user");
|
||||
|
||||
assert_eq!(cached.len(), 1);
|
||||
assert_eq!(cached[0].callable_name, "allowed_tool");
|
||||
assert_eq!(cached[0].connector_id.as_deref(), Some("calendar"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn codex_apps_tools_cache_keeps_plugin_declared_openai_connector() {
|
||||
let codex_home = tempdir().expect("tempdir");
|
||||
let cache_context = create_codex_apps_tools_cache_context(
|
||||
codex_home.path().to_path_buf(),
|
||||
Some("account-one"),
|
||||
Some("user-one"),
|
||||
);
|
||||
let tools = vec![
|
||||
create_test_tool_with_connector(
|
||||
CODEX_APPS_MCP_SERVER_NAME,
|
||||
"appgarden_tool",
|
||||
"connector_openai_appgarden",
|
||||
Some("Appgen"),
|
||||
),
|
||||
create_test_tool_with_connector(
|
||||
CODEX_APPS_MCP_SERVER_NAME,
|
||||
"hidden_tool",
|
||||
"connector_openai_hidden",
|
||||
Some("Hidden"),
|
||||
),
|
||||
];
|
||||
|
||||
write_cached_codex_apps_tools(&cache_context, &tools);
|
||||
let cached = read_cached_codex_apps_tools(
|
||||
&cache_context,
|
||||
&provenance_for_connector("connector_openai_appgarden"),
|
||||
)
|
||||
.expect("cache entry exists for user");
|
||||
|
||||
assert_eq!(cached.len(), 1);
|
||||
assert_eq!(cached[0].callable_name, "appgarden_tool");
|
||||
assert_eq!(
|
||||
cached[0].connector_id.as_deref(),
|
||||
Some("connector_openai_appgarden")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn codex_apps_tools_cache_hides_cached_openai_connector_without_current_provenance() {
|
||||
let codex_home = tempdir().expect("tempdir");
|
||||
let cache_context = create_codex_apps_tools_cache_context(
|
||||
codex_home.path().to_path_buf(),
|
||||
Some("account-one"),
|
||||
Some("user-one"),
|
||||
);
|
||||
let tools = vec![create_test_tool_with_connector(
|
||||
CODEX_APPS_MCP_SERVER_NAME,
|
||||
"appgarden_tool",
|
||||
"connector_openai_appgarden",
|
||||
Some("Appgen"),
|
||||
)];
|
||||
|
||||
write_cached_codex_apps_tools(&cache_context, &tools);
|
||||
let cached = read_cached_codex_apps_tools(&cache_context, &ToolPluginProvenance::default())
|
||||
.expect("cache entry exists for user");
|
||||
|
||||
assert!(cached.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn codex_apps_tools_cache_is_ignored_when_schema_version_mismatches() {
|
||||
let codex_home = tempdir().expect("tempdir");
|
||||
@@ -572,7 +647,9 @@ fn codex_apps_tools_cache_is_ignored_when_schema_version_mismatches() {
|
||||
.expect("serialize");
|
||||
std::fs::write(cache_path, bytes).expect("write");
|
||||
|
||||
assert!(read_cached_codex_apps_tools(&cache_context).is_none());
|
||||
assert!(
|
||||
read_cached_codex_apps_tools(&cache_context, &ToolPluginProvenance::default(),).is_none()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -589,7 +666,9 @@ fn codex_apps_tools_cache_is_ignored_when_json_is_invalid() {
|
||||
}
|
||||
std::fs::write(cache_path, b"{not json").expect("write");
|
||||
|
||||
assert!(read_cached_codex_apps_tools(&cache_context).is_none());
|
||||
assert!(
|
||||
read_cached_codex_apps_tools(&cache_context, &ToolPluginProvenance::default(),).is_none()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -609,6 +688,7 @@ fn startup_cached_codex_apps_tools_loads_from_disk_cache() {
|
||||
let startup_snapshot = load_startup_cached_codex_apps_tools_snapshot(
|
||||
CODEX_APPS_MCP_SERVER_NAME,
|
||||
Some(&cache_context),
|
||||
&ToolPluginProvenance::default(),
|
||||
);
|
||||
let startup_tools = startup_snapshot.expect("expected startup snapshot to load from cache");
|
||||
|
||||
|
||||
@@ -7,6 +7,7 @@ pub fn filter_tool_suggest_discoverable_connectors(
|
||||
accessible_connectors: &[AppInfo],
|
||||
discoverable_connector_ids: &HashSet<String>,
|
||||
originator_value: &str,
|
||||
plugin_declared_connector_ids: &HashSet<String>,
|
||||
) -> Vec<AppInfo> {
|
||||
let accessible_connector_ids: HashSet<&str> = accessible_connectors
|
||||
.iter()
|
||||
@@ -14,11 +15,15 @@ pub fn filter_tool_suggest_discoverable_connectors(
|
||||
.map(|connector| connector.id.as_str())
|
||||
.collect();
|
||||
|
||||
let mut connectors = filter_disallowed_connectors(directory_connectors, originator_value)
|
||||
.into_iter()
|
||||
.filter(|connector| !accessible_connector_ids.contains(connector.id.as_str()))
|
||||
.filter(|connector| discoverable_connector_ids.contains(connector.id.as_str()))
|
||||
.collect::<Vec<_>>();
|
||||
let mut connectors = filter_disallowed_connectors(
|
||||
directory_connectors,
|
||||
originator_value,
|
||||
plugin_declared_connector_ids,
|
||||
)
|
||||
.into_iter()
|
||||
.filter(|connector| !accessible_connector_ids.contains(connector.id.as_str()))
|
||||
.filter(|connector| discoverable_connector_ids.contains(connector.id.as_str()))
|
||||
.collect::<Vec<_>>();
|
||||
connectors.sort_by(|left, right| {
|
||||
left.name
|
||||
.cmp(&right.name)
|
||||
@@ -42,12 +47,16 @@ const DISALLOWED_CONNECTOR_PREFIX: &str = "connector_openai_";
|
||||
pub fn filter_disallowed_connectors(
|
||||
connectors: Vec<AppInfo>,
|
||||
originator_value: &str,
|
||||
plugin_declared_connector_ids: &HashSet<String>,
|
||||
) -> Vec<AppInfo> {
|
||||
let first_party_chat_originator = is_first_party_chat_originator(originator_value);
|
||||
connectors
|
||||
.into_iter()
|
||||
.filter(|connector| {
|
||||
is_connector_id_allowed(connector.id.as_str(), first_party_chat_originator)
|
||||
connector_id_passes_disallow_filter(
|
||||
connector.id.as_str(),
|
||||
originator_value,
|
||||
plugin_declared_connector_ids,
|
||||
)
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
@@ -56,13 +65,95 @@ fn is_first_party_chat_originator(originator_value: &str) -> bool {
|
||||
originator_value == "codex_atlas" || originator_value == "codex_chatgpt_desktop"
|
||||
}
|
||||
|
||||
fn is_connector_id_allowed(connector_id: &str, first_party_chat_originator: bool) -> bool {
|
||||
pub fn connector_id_passes_disallow_filter(
|
||||
connector_id: &str,
|
||||
originator_value: &str,
|
||||
plugin_declared_connector_ids: &HashSet<String>,
|
||||
) -> bool {
|
||||
let first_party_chat_originator = is_first_party_chat_originator(originator_value);
|
||||
let disallowed_connector_ids = if first_party_chat_originator {
|
||||
FIRST_PARTY_CHAT_DISALLOWED_CONNECTOR_IDS
|
||||
} else {
|
||||
DISALLOWED_CONNECTOR_IDS
|
||||
};
|
||||
|
||||
if disallowed_connector_ids.contains(&connector_id) {
|
||||
return false;
|
||||
}
|
||||
|
||||
!connector_id.starts_with(DISALLOWED_CONNECTOR_PREFIX)
|
||||
&& !disallowed_connector_ids.contains(&connector_id)
|
||||
|| plugin_declared_connector_ids.contains(connector_id)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
fn app(id: &str) -> AppInfo {
|
||||
AppInfo {
|
||||
id: id.to_string(),
|
||||
name: id.to_string(),
|
||||
description: None,
|
||||
logo_url: None,
|
||||
logo_url_dark: None,
|
||||
distribution_channel: None,
|
||||
branding: None,
|
||||
app_metadata: None,
|
||||
labels: None,
|
||||
install_url: None,
|
||||
is_accessible: false,
|
||||
is_enabled: true,
|
||||
plugin_display_names: Vec::new(),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filter_disallowed_connectors_filters_openai_prefix_without_plugin_provenance() {
|
||||
let filtered = filter_disallowed_connectors(
|
||||
vec![app("connector_openai_foo"), app("gamma")],
|
||||
"codex_cli",
|
||||
&HashSet::new(),
|
||||
);
|
||||
|
||||
assert_eq!(filtered, vec![app("gamma")]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filter_disallowed_connectors_allows_plugin_declared_openai_prefix() {
|
||||
let plugin_declared_connector_ids =
|
||||
HashSet::from(["connector_openai_appgarden".to_string()]);
|
||||
|
||||
let filtered = filter_disallowed_connectors(
|
||||
vec![
|
||||
app("connector_openai_appgarden"),
|
||||
app("connector_openai_hidden"),
|
||||
app("gamma"),
|
||||
],
|
||||
"codex_cli",
|
||||
&plugin_declared_connector_ids,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
filtered,
|
||||
vec![app("connector_openai_appgarden"), app("gamma")]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filter_disallowed_connectors_keeps_explicit_denylist_over_plugin_provenance() {
|
||||
let plugin_declared_connector_ids =
|
||||
HashSet::from(["asdk_app_6938a94a61d881918ef32cb999ff937c".to_string()]);
|
||||
|
||||
let filtered = filter_disallowed_connectors(
|
||||
vec![
|
||||
app("asdk_app_6938a94a61d881918ef32cb999ff937c"),
|
||||
app("gamma"),
|
||||
],
|
||||
"codex_cli",
|
||||
&plugin_declared_connector_ids,
|
||||
);
|
||||
|
||||
assert_eq!(filtered, vec![app("gamma")]);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -119,13 +119,15 @@ pub(crate) async fn list_tool_suggest_discoverable_tools_with_auth(
|
||||
) -> anyhow::Result<Vec<DiscoverableTool>> {
|
||||
let directory_connectors =
|
||||
list_directory_connectors_for_tool_suggest_with_auth(config, auth).await?;
|
||||
let connector_ids = tool_suggest_connector_ids(config).await;
|
||||
let plugin_declared_connector_ids = plugin_declared_connector_ids_for_config(config).await;
|
||||
let connector_ids = tool_suggest_connector_ids(config, &plugin_declared_connector_ids);
|
||||
let discoverable_connectors =
|
||||
codex_connectors::filter::filter_tool_suggest_discoverable_connectors(
|
||||
directory_connectors,
|
||||
accessible_connectors,
|
||||
&connector_ids,
|
||||
originator().value.as_str(),
|
||||
&plugin_declared_connector_ids,
|
||||
)
|
||||
.into_iter()
|
||||
.map(DiscoverableTool::from);
|
||||
@@ -151,15 +153,17 @@ pub async fn list_cached_accessible_connectors_from_mcp_tools(
|
||||
return Some(Vec::new());
|
||||
}
|
||||
let cache_key = accessible_connectors_cache_key(config, auth.as_ref());
|
||||
let plugin_declared_connector_ids = plugin_declared_connector_ids_for_config(config).await;
|
||||
read_cached_accessible_connectors(&cache_key).map(|connectors| {
|
||||
codex_connectors::filter::filter_disallowed_connectors(
|
||||
connectors,
|
||||
originator().value.as_str(),
|
||||
&plugin_declared_connector_ids,
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
pub(crate) fn refresh_accessible_connectors_cache_from_mcp_tools(
|
||||
pub(crate) async fn refresh_accessible_connectors_cache_from_mcp_tools(
|
||||
config: &Config,
|
||||
auth: Option<&CodexAuth>,
|
||||
mcp_tools: &HashMap<String, ToolInfo>,
|
||||
@@ -169,9 +173,11 @@ pub(crate) fn refresh_accessible_connectors_cache_from_mcp_tools(
|
||||
}
|
||||
|
||||
let cache_key = accessible_connectors_cache_key(config, auth);
|
||||
let plugin_declared_connector_ids = plugin_declared_connector_ids_for_config(config).await;
|
||||
let accessible_connectors = codex_connectors::filter::filter_disallowed_connectors(
|
||||
accessible_connectors_from_mcp_tools(mcp_tools),
|
||||
originator().value.as_str(),
|
||||
&plugin_declared_connector_ids,
|
||||
);
|
||||
write_cached_accessible_connectors(cache_key, &accessible_connectors);
|
||||
}
|
||||
@@ -207,11 +213,13 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_options_and_status(
|
||||
let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.to_path_buf()));
|
||||
let mcp_manager = McpManager::new(Arc::clone(&plugins_manager));
|
||||
let tool_plugin_provenance = mcp_manager.tool_plugin_provenance(config).await;
|
||||
let plugin_declared_connector_ids = tool_plugin_provenance.plugin_declared_connector_ids();
|
||||
if !force_refetch && let Some(cached_connectors) = read_cached_accessible_connectors(&cache_key)
|
||||
{
|
||||
let cached_connectors = codex_connectors::filter::filter_disallowed_connectors(
|
||||
cached_connectors,
|
||||
originator().value.as_str(),
|
||||
&plugin_declared_connector_ids,
|
||||
);
|
||||
let cached_connectors = with_app_plugin_sources(cached_connectors, &tool_plugin_provenance);
|
||||
return Ok(AccessibleConnectorsStatus {
|
||||
@@ -258,7 +266,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_options_and_status(
|
||||
McpRuntimeEnvironment::new(Arc::new(Environment::default()), config.cwd.to_path_buf()),
|
||||
config.codex_home.to_path_buf(),
|
||||
codex_apps_tools_cache_key(auth.as_ref()),
|
||||
ToolPluginProvenance::default(),
|
||||
tool_plugin_provenance.clone(),
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -319,6 +327,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_options_and_status(
|
||||
let accessible_connectors = codex_connectors::filter::filter_disallowed_connectors(
|
||||
accessible_connectors_from_mcp_tools(&tools),
|
||||
originator().value.as_str(),
|
||||
&plugin_declared_connector_ids,
|
||||
);
|
||||
if codex_apps_ready || !accessible_connectors.is_empty() {
|
||||
write_cached_accessible_connectors(cache_key, &accessible_connectors);
|
||||
@@ -387,15 +396,22 @@ fn write_cached_accessible_connectors(
|
||||
});
|
||||
}
|
||||
|
||||
async fn tool_suggest_connector_ids(config: &Config) -> HashSet<String> {
|
||||
let mut connector_ids = PluginsManager::new(config.codex_home.to_path_buf())
|
||||
async fn plugin_declared_connector_ids_for_config(config: &Config) -> HashSet<String> {
|
||||
PluginsManager::new(config.codex_home.to_path_buf())
|
||||
.plugins_for_config(config)
|
||||
.await
|
||||
.capability_summaries()
|
||||
.iter()
|
||||
.flat_map(|plugin| plugin.app_connector_ids.iter())
|
||||
.map(|connector_id| connector_id.0.clone())
|
||||
.collect::<HashSet<_>>();
|
||||
.collect::<HashSet<_>>()
|
||||
}
|
||||
|
||||
fn tool_suggest_connector_ids(
|
||||
config: &Config,
|
||||
plugin_declared_connector_ids: &HashSet<String>,
|
||||
) -> HashSet<String> {
|
||||
let mut connector_ids = plugin_declared_connector_ids.clone();
|
||||
connector_ids.extend(
|
||||
config
|
||||
.tool_suggest
|
||||
|
||||
@@ -28,8 +28,15 @@ use rmcp::model::Tool;
|
||||
use std::collections::BTreeMap;
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
use std::future::Future;
|
||||
use std::path::Path;
|
||||
use std::sync::Arc;
|
||||
use std::sync::LazyLock;
|
||||
use tempfile::tempdir;
|
||||
use tokio::sync::Mutex as TokioMutex;
|
||||
|
||||
static ACCESSIBLE_CONNECTORS_CACHE_TEST_LOCK: LazyLock<TokioMutex<()>> =
|
||||
LazyLock::new(|| TokioMutex::new(()));
|
||||
|
||||
fn annotations(destructive_hint: Option<bool>, open_world_hint: Option<bool>) -> ToolAnnotations {
|
||||
ToolAnnotations {
|
||||
@@ -128,14 +135,51 @@ fn codex_app_tool(
|
||||
}
|
||||
}
|
||||
|
||||
fn with_accessible_connectors_cache_cleared<R>(f: impl FnOnce() -> R) -> R {
|
||||
fn write_installed_plugin_app(codex_home: &Path, connector_id: &str) {
|
||||
let plugin_root = codex_home.join("plugins/cache/test/sample/local");
|
||||
std::fs::create_dir_all(plugin_root.join(".codex-plugin")).expect("create plugin manifest dir");
|
||||
std::fs::write(
|
||||
plugin_root.join(".codex-plugin/plugin.json"),
|
||||
r#"{"name":"sample"}"#,
|
||||
)
|
||||
.expect("write plugin manifest");
|
||||
std::fs::write(
|
||||
plugin_root.join(".app.json"),
|
||||
format!(
|
||||
r#"{{
|
||||
"apps": {{
|
||||
"sample": {{
|
||||
"id": "{connector_id}"
|
||||
}}
|
||||
}}
|
||||
}}"#
|
||||
),
|
||||
)
|
||||
.expect("write plugin app config");
|
||||
std::fs::write(
|
||||
codex_home.join(CONFIG_TOML_FILE),
|
||||
r#"[features]
|
||||
plugins = true
|
||||
|
||||
[plugins."sample@test"]
|
||||
enabled = true
|
||||
"#,
|
||||
)
|
||||
.expect("write config");
|
||||
}
|
||||
|
||||
async fn with_accessible_connectors_cache_cleared<R, F>(f: impl FnOnce() -> F) -> R
|
||||
where
|
||||
F: Future<Output = R>,
|
||||
{
|
||||
let _guard = ACCESSIBLE_CONNECTORS_CACHE_TEST_LOCK.lock().await;
|
||||
let previous = {
|
||||
let mut cache_guard = ACCESSIBLE_CONNECTORS_CACHE
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner);
|
||||
cache_guard.take()
|
||||
};
|
||||
let result = f();
|
||||
let result = f().await;
|
||||
let mut cache_guard = ACCESSIBLE_CONNECTORS_CACHE
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner);
|
||||
@@ -233,6 +277,7 @@ fn accessible_connectors_from_mcp_tools_carries_plugin_display_names() {
|
||||
#[tokio::test]
|
||||
async fn refresh_accessible_connectors_cache_from_mcp_tools_writes_latest_installed_apps() {
|
||||
let codex_home = tempdir().expect("tempdir should succeed");
|
||||
write_installed_plugin_app(codex_home.path(), "connector_openai_appgarden");
|
||||
let mut config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.build()
|
||||
@@ -250,6 +295,15 @@ async fn refresh_accessible_connectors_cache_from_mcp_tools_writes_latest_instal
|
||||
&["calendar-plugin"],
|
||||
),
|
||||
),
|
||||
(
|
||||
"mcp__codex_apps__appgarden_create_project".to_string(),
|
||||
codex_app_tool(
|
||||
"appgarden_create_project",
|
||||
"connector_openai_appgarden",
|
||||
Some("Appgen"),
|
||||
&["appgarden-plugin"],
|
||||
),
|
||||
),
|
||||
(
|
||||
"mcp__codex_apps__openai_hidden".to_string(),
|
||||
codex_app_tool(
|
||||
@@ -261,31 +315,92 @@ async fn refresh_accessible_connectors_cache_from_mcp_tools_writes_latest_instal
|
||||
),
|
||||
]);
|
||||
|
||||
let cached = with_accessible_connectors_cache_cleared(|| {
|
||||
refresh_accessible_connectors_cache_from_mcp_tools(&config, /*auth*/ None, &tools);
|
||||
let cached = with_accessible_connectors_cache_cleared(|| async {
|
||||
refresh_accessible_connectors_cache_from_mcp_tools(&config, /*auth*/ None, &tools).await;
|
||||
read_cached_accessible_connectors(&cache_key).expect("cache should be populated")
|
||||
});
|
||||
})
|
||||
.await;
|
||||
|
||||
assert_eq!(
|
||||
cached,
|
||||
vec![AppInfo {
|
||||
id: "calendar".to_string(),
|
||||
name: "Google Calendar".to_string(),
|
||||
description: None,
|
||||
logo_url: None,
|
||||
logo_url_dark: None,
|
||||
distribution_channel: None,
|
||||
install_url: Some(connector_install_url("Google Calendar", "calendar")),
|
||||
branding: None,
|
||||
app_metadata: None,
|
||||
labels: None,
|
||||
is_accessible: true,
|
||||
is_enabled: true,
|
||||
plugin_display_names: plugin_names(&["calendar-plugin"]),
|
||||
}]
|
||||
vec![
|
||||
AppInfo {
|
||||
id: "connector_openai_appgarden".to_string(),
|
||||
name: "Appgen".to_string(),
|
||||
description: None,
|
||||
logo_url: None,
|
||||
logo_url_dark: None,
|
||||
distribution_channel: None,
|
||||
install_url: Some(connector_install_url(
|
||||
"Appgen",
|
||||
"connector_openai_appgarden"
|
||||
)),
|
||||
branding: None,
|
||||
app_metadata: None,
|
||||
labels: None,
|
||||
is_accessible: true,
|
||||
is_enabled: true,
|
||||
plugin_display_names: plugin_names(&["appgarden-plugin"]),
|
||||
},
|
||||
AppInfo {
|
||||
id: "calendar".to_string(),
|
||||
name: "Google Calendar".to_string(),
|
||||
description: None,
|
||||
logo_url: None,
|
||||
logo_url_dark: None,
|
||||
distribution_channel: None,
|
||||
install_url: Some(connector_install_url("Google Calendar", "calendar")),
|
||||
branding: None,
|
||||
app_metadata: None,
|
||||
labels: None,
|
||||
is_accessible: true,
|
||||
is_enabled: true,
|
||||
plugin_display_names: plugin_names(&["calendar-plugin"]),
|
||||
},
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn list_cached_accessible_connectors_applies_current_plugin_provenance() {
|
||||
let codex_home = tempdir().expect("tempdir should succeed");
|
||||
let mut config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.build()
|
||||
.await
|
||||
.expect("config should load");
|
||||
let _ = config.features.set_enabled(Feature::Apps, /*enabled*/ true);
|
||||
let cache_key = accessible_connectors_cache_key(&config, /*auth*/ None);
|
||||
let cached_connectors = vec![AppInfo {
|
||||
id: "connector_openai_appgarden".to_string(),
|
||||
name: "Appgen".to_string(),
|
||||
description: None,
|
||||
logo_url: None,
|
||||
logo_url_dark: None,
|
||||
distribution_channel: None,
|
||||
install_url: Some(connector_install_url(
|
||||
"Appgen",
|
||||
"connector_openai_appgarden",
|
||||
)),
|
||||
branding: None,
|
||||
app_metadata: None,
|
||||
labels: None,
|
||||
is_accessible: true,
|
||||
is_enabled: true,
|
||||
plugin_display_names: Vec::new(),
|
||||
}];
|
||||
|
||||
let filtered = with_accessible_connectors_cache_cleared(|| async {
|
||||
write_cached_accessible_connectors(cache_key, &cached_connectors);
|
||||
list_cached_accessible_connectors_from_mcp_tools(&config)
|
||||
.await
|
||||
.expect("cache should be readable")
|
||||
})
|
||||
.await;
|
||||
|
||||
assert_eq!(filtered, Vec::<AppInfo>::new());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn merge_connectors_unions_and_dedupes_plugin_display_names() {
|
||||
let mut plugin = plugin_connector_to_app_info("calendar".to_string());
|
||||
@@ -1038,8 +1153,11 @@ fn app_tool_policy_matches_prefix_stripped_tool_name_for_tool_config() {
|
||||
|
||||
#[test]
|
||||
fn filter_disallowed_connectors_allows_non_disallowed_connectors() {
|
||||
let filtered =
|
||||
filter_disallowed_connectors(vec![app("asdk_app_hidden"), app("alpha")], "codex_cli");
|
||||
let filtered = filter_disallowed_connectors(
|
||||
vec![app("asdk_app_hidden"), app("alpha")],
|
||||
"codex_cli",
|
||||
&HashSet::new(),
|
||||
);
|
||||
assert_eq!(filtered, vec![app("asdk_app_hidden"), app("alpha")]);
|
||||
}
|
||||
|
||||
@@ -1052,6 +1170,7 @@ fn filter_disallowed_connectors_filters_openai_prefix() {
|
||||
app("gamma"),
|
||||
],
|
||||
"codex_cli",
|
||||
&HashSet::new(),
|
||||
);
|
||||
assert_eq!(filtered, vec![app("gamma")]);
|
||||
}
|
||||
@@ -1065,6 +1184,7 @@ fn filter_disallowed_connectors_filters_disallowed_connector_ids() {
|
||||
app("delta"),
|
||||
],
|
||||
"codex_cli",
|
||||
&HashSet::new(),
|
||||
);
|
||||
assert_eq!(filtered, vec![app("delta")]);
|
||||
}
|
||||
@@ -1078,6 +1198,7 @@ fn first_party_chat_originator_filters_target_and_openai_prefixed_connectors() {
|
||||
app("connector_0f9c9d4592e54d0a9a12b3f44a1e2010"),
|
||||
],
|
||||
"codex_atlas",
|
||||
&HashSet::new(),
|
||||
);
|
||||
assert_eq!(
|
||||
filtered,
|
||||
@@ -1107,7 +1228,7 @@ discoverables = [
|
||||
.expect("config should load");
|
||||
|
||||
assert_eq!(
|
||||
tool_suggest_connector_ids(&config).await,
|
||||
tool_suggest_connector_ids(&config, &HashSet::new()),
|
||||
HashSet::from(["connector_2128aebfecb84f64a069897515042a44".to_string()])
|
||||
);
|
||||
}
|
||||
@@ -1135,6 +1256,7 @@ fn filter_tool_suggest_discoverable_connectors_keeps_only_plugin_backed_uninstal
|
||||
"connector_68df038e0ba48191908c8434991bbac2".to_string(),
|
||||
]),
|
||||
"codex_cli",
|
||||
&HashSet::new(),
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
@@ -1146,6 +1268,41 @@ fn filter_tool_suggest_discoverable_connectors_keeps_only_plugin_backed_uninstal
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filter_tool_suggest_discoverable_connectors_allows_plugin_declared_openai_prefix() {
|
||||
let filtered = filter_tool_suggest_discoverable_connectors(
|
||||
vec![
|
||||
named_app("connector_openai_appgarden", "Appgen"),
|
||||
named_app("connector_openai_hidden", "Hidden"),
|
||||
],
|
||||
&[],
|
||||
&HashSet::from([
|
||||
"connector_openai_appgarden".to_string(),
|
||||
"connector_openai_hidden".to_string(),
|
||||
]),
|
||||
"codex_cli",
|
||||
&HashSet::from(["connector_openai_appgarden".to_string()]),
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
filtered,
|
||||
vec![named_app("connector_openai_appgarden", "Appgen")]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filter_tool_suggest_discoverable_connectors_blocks_config_only_openai_prefix() {
|
||||
let filtered = filter_tool_suggest_discoverable_connectors(
|
||||
vec![named_app("connector_openai_hidden", "Hidden")],
|
||||
&[],
|
||||
&HashSet::from(["connector_openai_hidden".to_string()]),
|
||||
"codex_cli",
|
||||
&HashSet::new(),
|
||||
);
|
||||
|
||||
assert_eq!(filtered, Vec::<AppInfo>::new());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filter_tool_suggest_discoverable_connectors_excludes_accessible_apps_even_when_disabled() {
|
||||
let filtered = filter_tool_suggest_discoverable_connectors(
|
||||
@@ -1175,6 +1332,7 @@ fn filter_tool_suggest_discoverable_connectors_excludes_accessible_apps_even_whe
|
||||
"connector_68df038e0ba48191908c8434991bbac2".to_string(),
|
||||
]),
|
||||
"codex_cli",
|
||||
&HashSet::new(),
|
||||
);
|
||||
|
||||
assert_eq!(filtered, Vec::<AppInfo>::new());
|
||||
|
||||
@@ -232,7 +232,8 @@ async fn refresh_missing_suggested_connectors(
|
||||
&turn.config,
|
||||
auth,
|
||||
&mcp_tools,
|
||||
);
|
||||
)
|
||||
.await;
|
||||
Some(accessible_connectors)
|
||||
}
|
||||
Err(err) => {
|
||||
|
||||
@@ -7830,12 +7830,15 @@ impl ChatWidget {
|
||||
});
|
||||
|
||||
let result: Result<ConnectorsSnapshot, String> = async {
|
||||
let plugin_declared_connector_ids =
|
||||
connectors::plugin_declared_connector_ids_for_config(&config).await;
|
||||
let all_connectors =
|
||||
connectors::list_all_connectors_with_options(&config, force_refetch).await?;
|
||||
let connectors = connectors::merge_connectors_with_accessible(
|
||||
all_connectors,
|
||||
accessible_connectors,
|
||||
/*all_connectors_loaded*/ true,
|
||||
&plugin_declared_connector_ids,
|
||||
);
|
||||
Ok(ConnectorsSnapshot { connectors })
|
||||
}
|
||||
@@ -10902,13 +10905,6 @@ impl ChatWidget {
|
||||
|
||||
match result {
|
||||
Ok(mut snapshot) => {
|
||||
if !is_final {
|
||||
snapshot.connectors = connectors::merge_connectors_with_accessible(
|
||||
Vec::new(),
|
||||
snapshot.connectors,
|
||||
/*all_connectors_loaded*/ false,
|
||||
);
|
||||
}
|
||||
snapshot.connectors =
|
||||
connectors::with_app_enabled_state(snapshot.connectors, &self.config);
|
||||
if let ConnectorsCacheState::Ready(existing_snapshot) = &self.connectors_cache {
|
||||
|
||||
@@ -15,7 +15,6 @@ workspace = true
|
||||
|
||||
[dependencies]
|
||||
codex-exec-server = { workspace = true }
|
||||
codex-login = { workspace = true }
|
||||
codex-utils-absolute-path = { workspace = true }
|
||||
serde = { workspace = true, features = ["derive"] }
|
||||
serde_json = { workspace = true }
|
||||
|
||||
@@ -1,33 +1,3 @@
|
||||
use codex_login::default_client::is_first_party_chat_originator;
|
||||
use codex_login::default_client::originator;
|
||||
|
||||
const DISALLOWED_CONNECTOR_IDS: &[&str] = &[
|
||||
"asdk_app_6938a94a61d881918ef32cb999ff937c",
|
||||
"connector_2b0a9009c9c64bf9933a3dae3f2b1254",
|
||||
"connector_3f8d1a79f27c4c7ba1a897ab13bf37dc",
|
||||
"connector_68de829bf7648191acd70a907364c67c",
|
||||
"connector_68e004f14af881919eb50893d3d9f523",
|
||||
"connector_69272cb413a081919685ec3c88d1744e",
|
||||
];
|
||||
const FIRST_PARTY_CHAT_DISALLOWED_CONNECTOR_IDS: &[&str] =
|
||||
&["connector_0f9c9d4592e54d0a9a12b3f44a1e2010"];
|
||||
const DISALLOWED_CONNECTOR_PREFIX: &str = "connector_openai_";
|
||||
|
||||
pub fn is_connector_id_allowed(connector_id: &str) -> bool {
|
||||
is_connector_id_allowed_for_originator(connector_id, originator().value.as_str())
|
||||
}
|
||||
|
||||
fn is_connector_id_allowed_for_originator(connector_id: &str, originator_value: &str) -> bool {
|
||||
let disallowed_connector_ids = if is_first_party_chat_originator(originator_value) {
|
||||
FIRST_PARTY_CHAT_DISALLOWED_CONNECTOR_IDS
|
||||
} else {
|
||||
DISALLOWED_CONNECTOR_IDS
|
||||
};
|
||||
|
||||
!connector_id.starts_with(DISALLOWED_CONNECTOR_PREFIX)
|
||||
&& !disallowed_connector_ids.contains(&connector_id)
|
||||
}
|
||||
|
||||
pub fn sanitize_name(name: &str) -> String {
|
||||
sanitize_slug(name).replace("-", "_")
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user