mirror of
https://github.com/openai/codex.git
synced 2026-10-21 17:48:44 +00:00
fix extension-managed Apps cache isolation
This commit is contained in:
@@ -66,10 +66,9 @@ impl AppsRequestProcessor {
|
||||
}
|
||||
|
||||
let auth = self.auth_manager.auth().await;
|
||||
if !config
|
||||
.features
|
||||
.apps_enabled_for_auth(auth.as_ref().is_some_and(CodexAuth::uses_codex_backend))
|
||||
{
|
||||
let mcp_manager = self.thread_manager.mcp_manager();
|
||||
let mcp_config = mcp_manager.runtime_config(&config).await;
|
||||
if !codex_mcp::host_owned_codex_apps_enabled(&mcp_config, auth.as_ref()) {
|
||||
return Ok(Some(AppsListResponse {
|
||||
data: Vec::new(),
|
||||
next_cursor: None,
|
||||
@@ -89,7 +88,6 @@ impl AppsRequestProcessor {
|
||||
let request = request_id.clone();
|
||||
let outgoing = Arc::clone(&self.outgoing);
|
||||
let environment_manager = self.thread_manager.environment_manager();
|
||||
let mcp_manager = self.thread_manager.mcp_manager();
|
||||
let plugins_manager = self.thread_manager.plugins_manager();
|
||||
let shutdown_token = self.shutdown_token.child_token();
|
||||
tokio::spawn(async move {
|
||||
@@ -192,7 +190,10 @@ impl AppsRequestProcessor {
|
||||
);
|
||||
let plugin_apps = connector_snapshot.connector_ids().to_vec();
|
||||
let (mut accessible_connectors, mut all_connectors) = tokio::join!(
|
||||
connectors::list_cached_accessible_connectors_from_mcp_tools(&config),
|
||||
connectors::list_cached_accessible_connectors_from_mcp_tools(
|
||||
&config,
|
||||
mcp_manager.as_ref(),
|
||||
),
|
||||
connectors::list_cached_all_connectors(&config, &plugin_apps)
|
||||
);
|
||||
let cached_all_connectors = all_connectors.clone();
|
||||
|
||||
@@ -1764,13 +1764,14 @@ impl PluginRequestProcessor {
|
||||
}
|
||||
|
||||
let environment_manager = self.thread_manager.environment_manager();
|
||||
let mcp_manager = self.thread_manager.mcp_manager();
|
||||
let (all_connectors_result, accessible_connectors_result) = tokio::join!(
|
||||
connectors::list_all_connectors_with_options(config, /*force_refetch*/ false, &[]),
|
||||
connectors::list_accessible_connectors_from_mcp_tools_with_mcp_manager(
|
||||
config,
|
||||
/*force_refetch*/ true,
|
||||
Arc::clone(&environment_manager),
|
||||
self.thread_manager.mcp_manager(),
|
||||
Arc::clone(&mcp_manager),
|
||||
),
|
||||
);
|
||||
|
||||
@@ -1795,9 +1796,12 @@ impl PluginRequestProcessor {
|
||||
"failed to load accessible apps after plugin install: {err:#}"
|
||||
);
|
||||
(
|
||||
connectors::list_cached_accessible_connectors_from_mcp_tools(config)
|
||||
.await
|
||||
.unwrap_or_default(),
|
||||
connectors::list_cached_accessible_connectors_from_mcp_tools(
|
||||
config,
|
||||
mcp_manager.as_ref(),
|
||||
)
|
||||
.await
|
||||
.unwrap_or_default(),
|
||||
false,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -136,6 +136,7 @@ impl McpConnectionManager {
|
||||
codex_home: PathBuf,
|
||||
codex_apps_tools_cache: CodexAppsToolsCache,
|
||||
codex_apps_tools_cache_key: CodexAppsToolsCacheKey,
|
||||
enable_codex_apps_tools_cache: bool,
|
||||
prefix_mcp_tool_names: bool,
|
||||
client_elicitation_capability: ElicitationCapability,
|
||||
supports_openai_form_elicitation: bool,
|
||||
@@ -192,8 +193,11 @@ impl McpConnectionManager {
|
||||
} => bearer_token_env_var.is_some(),
|
||||
McpServerTransportConfig::Stdio { .. } => false,
|
||||
});
|
||||
let shares_codex_apps_tools_cache =
|
||||
should_share_codex_apps_tools_cache(&server_name, uses_env_bearer_token);
|
||||
let shares_codex_apps_tools_cache = should_share_codex_apps_tools_cache(
|
||||
&server_name,
|
||||
uses_env_bearer_token,
|
||||
enable_codex_apps_tools_cache,
|
||||
);
|
||||
let codex_apps_tools_cache_context = shares_codex_apps_tools_cache.then(|| {
|
||||
codex_apps_tools_cache
|
||||
.context(codex_home.clone(), codex_apps_tools_cache_key.clone())
|
||||
@@ -913,8 +917,12 @@ fn chatgpt_auth_provider_for_server(
|
||||
chatgpt_auth_provider
|
||||
}
|
||||
|
||||
fn should_share_codex_apps_tools_cache(server_name: &str, uses_env_bearer_token: bool) -> bool {
|
||||
server_name == CODEX_APPS_MCP_SERVER_NAME && !uses_env_bearer_token
|
||||
fn should_share_codex_apps_tools_cache(
|
||||
server_name: &str,
|
||||
uses_env_bearer_token: bool,
|
||||
cache_enabled: bool,
|
||||
) -> bool {
|
||||
server_name == CODEX_APPS_MCP_SERVER_NAME && !uses_env_bearer_token && cache_enabled
|
||||
}
|
||||
|
||||
async fn emit_update(
|
||||
|
||||
@@ -735,6 +735,25 @@ fn codex_apps_env_bearer_token_bypasses_shared_tools_cache() {
|
||||
assert!(!should_share_codex_apps_tools_cache(
|
||||
CODEX_APPS_MCP_SERVER_NAME,
|
||||
/*uses_env_bearer_token*/ true,
|
||||
/*cache_enabled*/ true,
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn extension_managed_codex_apps_bypass_shared_tools_cache() {
|
||||
assert!(!should_share_codex_apps_tools_cache(
|
||||
CODEX_APPS_MCP_SERVER_NAME,
|
||||
/*uses_env_bearer_token*/ false,
|
||||
/*cache_enabled*/ false,
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn codex_apps_chatgpt_auth_uses_shared_tools_cache() {
|
||||
assert!(should_share_codex_apps_tools_cache(
|
||||
CODEX_APPS_MCP_SERVER_NAME,
|
||||
/*uses_env_bearer_token*/ false,
|
||||
/*cache_enabled*/ true,
|
||||
));
|
||||
}
|
||||
|
||||
@@ -1533,6 +1552,7 @@ async fn no_local_runtime_fails_local_stdio_but_keeps_local_http_server() {
|
||||
chatgpt_user_id: None,
|
||||
is_workspace_account: false,
|
||||
},
|
||||
/*enable_codex_apps_tools_cache*/ true,
|
||||
/*prefix_mcp_tool_names*/ true,
|
||||
ElicitationCapability::default(),
|
||||
/*supports_openai_form_elicitation*/ false,
|
||||
|
||||
@@ -348,6 +348,7 @@ pub async fn read_mcp_resource(
|
||||
config.codex_home.clone(),
|
||||
codex_apps_tools_cache,
|
||||
codex_apps_tools_cache_key(auth),
|
||||
/*enable_codex_apps_tools_cache*/ !extension_managed_codex_apps_enabled(config),
|
||||
config.prefix_mcp_tool_names,
|
||||
config.client_elicitation_capability.clone(),
|
||||
/*supports_openai_form_elicitation*/ false,
|
||||
@@ -425,6 +426,7 @@ pub async fn collect_mcp_server_status_snapshot_with_detail(
|
||||
config.codex_home.clone(),
|
||||
codex_apps_tools_cache,
|
||||
codex_apps_tools_cache_key(auth),
|
||||
/*enable_codex_apps_tools_cache*/ !extension_managed_codex_apps_enabled(config),
|
||||
config.prefix_mcp_tool_names,
|
||||
config.client_elicitation_capability.clone(),
|
||||
/*supports_openai_form_elicitation*/ false,
|
||||
|
||||
@@ -40,6 +40,8 @@ use codex_mcp::ToolPluginProvenance;
|
||||
use codex_mcp::codex_apps_tools_cache_key;
|
||||
use codex_mcp::compute_auth_statuses;
|
||||
use codex_mcp::effective_mcp_servers;
|
||||
use codex_mcp::extension_managed_codex_apps_enabled;
|
||||
use codex_mcp::host_owned_codex_apps_enabled;
|
||||
use codex_mcp::tool_plugin_provenance;
|
||||
|
||||
const CONNECTORS_READY_TIMEOUT_ON_EMPTY_TOOLS: Duration = Duration::from_secs(30);
|
||||
@@ -130,16 +132,18 @@ pub(crate) async fn list_tool_suggest_discoverable_tools_with_auth(
|
||||
|
||||
pub async fn list_cached_accessible_connectors_from_mcp_tools(
|
||||
config: &Config,
|
||||
mcp_manager: &McpManager,
|
||||
) -> Option<Vec<AppInfo>> {
|
||||
let auth_manager =
|
||||
AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false).await;
|
||||
let auth = auth_manager.auth().await;
|
||||
if !config
|
||||
.features
|
||||
.apps_enabled_for_auth(auth.as_ref().is_some_and(CodexAuth::uses_codex_backend))
|
||||
{
|
||||
let mcp_config = mcp_manager.runtime_config(config).await;
|
||||
if !host_owned_codex_apps_enabled(&mcp_config, auth.as_ref()) {
|
||||
return Some(Vec::new());
|
||||
}
|
||||
if extension_managed_codex_apps_enabled(&mcp_config) {
|
||||
return None;
|
||||
}
|
||||
let cache_key = accessible_connectors_cache_key(config, auth.as_ref());
|
||||
read_cached_accessible_connectors(&cache_key)
|
||||
}
|
||||
@@ -147,9 +151,10 @@ pub async fn list_cached_accessible_connectors_from_mcp_tools(
|
||||
pub(crate) fn refresh_accessible_connectors_cache_from_mcp_tools(
|
||||
config: &Config,
|
||||
auth: Option<&CodexAuth>,
|
||||
extension_managed_apps_enabled: bool,
|
||||
mcp_tools: &[ToolInfo],
|
||||
) {
|
||||
if !config.features.enabled(Feature::Apps) {
|
||||
if !config.features.enabled(Feature::Apps) || extension_managed_apps_enabled {
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -216,19 +221,19 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_mcp_manager(
|
||||
let auth_manager =
|
||||
AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false).await;
|
||||
let auth = auth_manager.auth().await;
|
||||
if !config
|
||||
.features
|
||||
.apps_enabled_for_auth(auth.as_ref().is_some_and(CodexAuth::uses_codex_backend))
|
||||
{
|
||||
let mcp_config = mcp_manager.runtime_config(config).await;
|
||||
if !host_owned_codex_apps_enabled(&mcp_config, auth.as_ref()) {
|
||||
return Ok(AccessibleConnectorsStatus {
|
||||
connectors: Vec::new(),
|
||||
codex_apps_ready: true,
|
||||
});
|
||||
}
|
||||
let cache_key = accessible_connectors_cache_key(config, auth.as_ref());
|
||||
let mcp_config = mcp_manager.runtime_config(config).await;
|
||||
let cache_key = (!extension_managed_codex_apps_enabled(&mcp_config))
|
||||
.then(|| accessible_connectors_cache_key(config, auth.as_ref()));
|
||||
let tool_plugin_provenance = tool_plugin_provenance(&mcp_config);
|
||||
if !force_refetch && let Some(cached_connectors) = read_cached_accessible_connectors(&cache_key)
|
||||
if !force_refetch
|
||||
&& let Some(cache_key) = cache_key.as_ref()
|
||||
&& let Some(cached_connectors) = read_cached_accessible_connectors(cache_key)
|
||||
{
|
||||
let cached_connectors = with_app_plugin_sources(cached_connectors, &tool_plugin_provenance);
|
||||
return Ok(AccessibleConnectorsStatus {
|
||||
@@ -277,6 +282,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_mcp_manager(
|
||||
config.codex_home.to_path_buf(),
|
||||
mcp_manager.codex_apps_tools_cache(),
|
||||
codex_apps_tools_cache_key(auth.as_ref()),
|
||||
/*enable_codex_apps_tools_cache*/ !extension_managed_codex_apps_enabled(&mcp_config),
|
||||
mcp_config.prefix_mcp_tool_names,
|
||||
mcp_config.client_elicitation_capability,
|
||||
/*supports_openai_form_elicitation*/ false,
|
||||
@@ -343,7 +349,9 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_mcp_manager(
|
||||
}
|
||||
|
||||
let accessible_connectors = accessible_connectors_for_app_list_from_mcp_tools(&tools);
|
||||
if codex_apps_ready || !accessible_connectors.is_empty() {
|
||||
if (codex_apps_ready || !accessible_connectors.is_empty())
|
||||
&& let Some(cache_key) = cache_key
|
||||
{
|
||||
write_cached_accessible_connectors(cache_key, &accessible_connectors);
|
||||
}
|
||||
let accessible_connectors =
|
||||
|
||||
@@ -6,6 +6,7 @@ use codex_config::AppsRequirementsToml;
|
||||
use codex_config::ConfigLayerStack;
|
||||
use codex_config::ConfigRequirements;
|
||||
use codex_config::ConfigRequirementsToml;
|
||||
use codex_config::McpServerAuth;
|
||||
use codex_config::test_support::CloudConfigBundleFixture;
|
||||
use codex_config::types::ApprovalsReviewer;
|
||||
use codex_connectors::merge::plugin_connector_to_app_info;
|
||||
@@ -92,6 +93,41 @@ fn with_accessible_connectors_cache_cleared<R>(f: impl FnOnce() -> R) -> R {
|
||||
result
|
||||
}
|
||||
|
||||
struct ExtensionManagedCodexApps;
|
||||
|
||||
impl codex_extension_api::McpServerContributor<Config> for ExtensionManagedCodexApps {
|
||||
fn id(&self) -> &'static str {
|
||||
"extension_managed_codex_apps"
|
||||
}
|
||||
|
||||
fn contribute<'a>(
|
||||
&'a self,
|
||||
context: codex_extension_api::McpServerContributionContext<'a, Config>,
|
||||
) -> codex_extension_api::ExtensionFuture<'a, Vec<codex_extension_api::McpServerContribution>>
|
||||
{
|
||||
Box::pin(async move {
|
||||
let mut server = codex_mcp::codex_apps_mcp_server_config(
|
||||
&context.config().chatgpt_base_url,
|
||||
/*apps_mcp_product_sku*/ None,
|
||||
);
|
||||
server.auth = McpServerAuth::OAuth;
|
||||
vec![codex_extension_api::McpServerContribution::Set {
|
||||
name: CODEX_APPS_MCP_SERVER_NAME.to_string(),
|
||||
config: Box::new(server),
|
||||
}]
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
fn extension_managed_mcp_manager(config: &Config) -> McpManager {
|
||||
let mut builder = codex_extension_api::ExtensionRegistryBuilder::new();
|
||||
builder.mcp_server_contributor(Arc::new(ExtensionManagedCodexApps));
|
||||
McpManager::new_with_extensions(
|
||||
Arc::new(PluginsManager::new(config.codex_home.to_path_buf())),
|
||||
Arc::new(builder.build()),
|
||||
)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn accessible_connectors_from_mcp_tools_carries_plugin_display_names() {
|
||||
let tools = vec![
|
||||
@@ -238,7 +274,9 @@ 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);
|
||||
refresh_accessible_connectors_cache_from_mcp_tools(
|
||||
&config, /*auth*/ None, /*extension_managed_apps_enabled*/ false, &tools,
|
||||
);
|
||||
read_cached_accessible_connectors(&cache_key).expect("cache should be populated")
|
||||
});
|
||||
|
||||
@@ -283,6 +321,69 @@ async fn refresh_accessible_connectors_cache_from_mcp_tools_writes_latest_instal
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn extension_managed_apps_do_not_write_accessible_connectors_cache() {
|
||||
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 tools = vec![codex_app_tool(
|
||||
"calendar_list_events",
|
||||
"calendar",
|
||||
Some("Google Calendar"),
|
||||
&[],
|
||||
)];
|
||||
|
||||
let cached = with_accessible_connectors_cache_cleared(|| {
|
||||
refresh_accessible_connectors_cache_from_mcp_tools(
|
||||
&config, /*auth*/ None, /*extension_managed_apps_enabled*/ true, &tools,
|
||||
);
|
||||
read_cached_accessible_connectors(&cache_key)
|
||||
});
|
||||
|
||||
assert_eq!(cached, None);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn extension_managed_apps_bypass_cached_connector_lookup_without_chatgpt_auth() {
|
||||
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 mcp_manager = extension_managed_mcp_manager(&config);
|
||||
|
||||
assert_eq!(
|
||||
list_cached_accessible_connectors_from_mcp_tools(&config, &mcp_manager).await,
|
||||
None,
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn compatibility_apps_remain_hidden_without_chatgpt_auth() {
|
||||
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 mcp_manager = McpManager::new(Arc::new(PluginsManager::new(
|
||||
config.codex_home.to_path_buf(),
|
||||
)));
|
||||
|
||||
assert_eq!(
|
||||
list_cached_accessible_connectors_from_mcp_tools(&config, &mcp_manager).await,
|
||||
Some(Vec::new()),
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn accessible_connectors_from_mcp_tools_preserves_description() {
|
||||
let mcp_tools = vec![ToolInfo {
|
||||
|
||||
@@ -700,6 +700,7 @@ async fn refresh_codex_apps_after_connector_auth(
|
||||
connectors::refresh_accessible_connectors_cache_from_mcp_tools(
|
||||
&turn_context.config,
|
||||
auth.as_ref(),
|
||||
turn_context.extension_managed_apps_enabled,
|
||||
&mcp_tools,
|
||||
);
|
||||
}
|
||||
@@ -1489,6 +1490,7 @@ pub(crate) async fn lookup_mcp_tool_metadata(
|
||||
let connector_description = if server == CODEX_APPS_MCP_SERVER_NAME {
|
||||
let connectors = match connectors::list_cached_accessible_connectors_from_mcp_tools(
|
||||
turn_context.config.as_ref(),
|
||||
sess.services.mcp_manager.as_ref(),
|
||||
)
|
||||
.await
|
||||
{
|
||||
|
||||
@@ -1433,6 +1433,7 @@ async fn host_owned_codex_apps_manager(
|
||||
turn_context.config.codex_home.to_path_buf(),
|
||||
session.services.mcp_manager.codex_apps_tools_cache(),
|
||||
codex_mcp::codex_apps_tools_cache_key(auth.as_ref()),
|
||||
/*enable_codex_apps_tools_cache*/ true,
|
||||
turn_context.config.prefix_mcp_tool_names(),
|
||||
rmcp::model::ElicitationCapability::default(),
|
||||
/*supports_openai_form_elicitation*/ false,
|
||||
|
||||
@@ -361,6 +361,8 @@ impl Session {
|
||||
mcp_config.codex_home.clone(),
|
||||
self.services.mcp_manager.codex_apps_tools_cache(),
|
||||
codex_apps_tools_cache_key(auth.as_ref()),
|
||||
/*enable_codex_apps_tools_cache*/
|
||||
!codex_mcp::extension_managed_codex_apps_enabled(&mcp_config),
|
||||
mcp_config.prefix_mcp_tool_names,
|
||||
mcp_config.client_elicitation_capability.clone(),
|
||||
self.services
|
||||
|
||||
@@ -1207,6 +1207,9 @@ impl Session {
|
||||
config.codex_home.to_path_buf(),
|
||||
sess.services.mcp_manager.codex_apps_tools_cache(),
|
||||
codex_apps_tools_cache_key(auth),
|
||||
/*enable_codex_apps_tools_cache*/ !codex_mcp::extension_managed_codex_apps_enabled(
|
||||
&mcp_config,
|
||||
),
|
||||
config.prefix_mcp_tool_names(),
|
||||
mcp_config.client_elicitation_capability.clone(),
|
||||
sess.services
|
||||
|
||||
@@ -468,6 +468,7 @@ async fn refresh_missing_requested_connectors(
|
||||
connectors::refresh_accessible_connectors_cache_from_mcp_tools(
|
||||
&turn.config,
|
||||
auth,
|
||||
turn.extension_managed_apps_enabled,
|
||||
&mcp_tools,
|
||||
);
|
||||
Some(accessible_connectors)
|
||||
|
||||
Reference in New Issue
Block a user