mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Preserve plugin apps in connector listings (#27602)
## Context This is PR4 in the plugin auth-routing stack. The earlier PRs make plugin surface projection auth-aware and narrow App/MCP conflicts by App declaration name. This PR keeps connector listing paths aligned with that projected plugin App set. This means ChatGPT/SIWC users will still see plugin-provided Apps in connector listing surfaces like the Apps/connector picker, while API-key users will not see Apps they cannot use. ## Stack - PR1: #27652 seed plugin manager auth at construction. - PR2: #27459 route plugin surfaces by auth mode. - PR3: #27607 dedupe plugin MCP servers by App declaration name. - PR4: #27602 preserve plugin Apps in connector listings. - PR5: #27461 skip install-time plugin MCP OAuth for matching App routes. ## Summary - Have app-server compute effective plugin Apps from the existing PluginsManager and pass them into connector listing. - Keep plugin Apps visible in Apps/connector listing for ChatGPT/SIWC users. - Keep API-key-style auth from surfacing plugin Apps in connector listings. ## Validation ```bash cargo test -p codex-chatgpt connectors::tests cargo test -p codex-app-server list_apps_includes_plugin_apps_for_chatgpt_auth git diff --check ```
This commit is contained in:
committed by
GitHub
Unverified
parent
127224cacc
commit
cededa26c5
Generated
-1
@@ -2257,7 +2257,6 @@ dependencies = [
|
||||
"codex-app-server-protocol",
|
||||
"codex-connectors",
|
||||
"codex-core",
|
||||
"codex-core-plugins",
|
||||
"codex-git-utils",
|
||||
"codex-login",
|
||||
"codex-model-provider",
|
||||
|
||||
@@ -318,6 +318,7 @@ use codex_core_plugins::PluginInstallError as CorePluginInstallError;
|
||||
use codex_core_plugins::PluginInstallRequest;
|
||||
use codex_core_plugins::PluginReadRequest;
|
||||
use codex_core_plugins::PluginUninstallError as CorePluginUninstallError;
|
||||
use codex_core_plugins::PluginsManager;
|
||||
use codex_core_plugins::loader::load_plugin_apps;
|
||||
use codex_core_plugins::loader::load_plugin_mcp_servers;
|
||||
use codex_core_plugins::loader::plugin_telemetry_metadata_from_root;
|
||||
|
||||
@@ -89,6 +89,7 @@ impl AppsRequestProcessor {
|
||||
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 {
|
||||
tokio::select! {
|
||||
@@ -100,6 +101,7 @@ impl AppsRequestProcessor {
|
||||
config,
|
||||
environment_manager,
|
||||
mcp_manager,
|
||||
plugins_manager,
|
||||
) => {}
|
||||
}
|
||||
});
|
||||
@@ -117,14 +119,22 @@ impl AppsRequestProcessor {
|
||||
config: Config,
|
||||
environment_manager: Arc<EnvironmentManager>,
|
||||
mcp_manager: Arc<McpManager>,
|
||||
plugins_manager: Arc<PluginsManager>,
|
||||
) {
|
||||
let retry_params = params.clone();
|
||||
let retry_config = config.clone();
|
||||
let retry_environment_manager = Arc::clone(&environment_manager);
|
||||
let retry_mcp_manager = Arc::clone(&mcp_manager);
|
||||
let result =
|
||||
Self::apps_list_response(&outgoing, params, config, environment_manager, mcp_manager)
|
||||
.await;
|
||||
let retry_plugins_manager = Arc::clone(&plugins_manager);
|
||||
let result = Self::apps_list_response(
|
||||
&outgoing,
|
||||
params,
|
||||
config,
|
||||
environment_manager,
|
||||
mcp_manager,
|
||||
plugins_manager,
|
||||
)
|
||||
.await;
|
||||
let should_retry = result
|
||||
.as_ref()
|
||||
.is_ok_and(|(_, codex_apps_ready)| !codex_apps_ready);
|
||||
@@ -141,6 +151,7 @@ impl AppsRequestProcessor {
|
||||
retry_config,
|
||||
retry_environment_manager,
|
||||
retry_mcp_manager,
|
||||
retry_plugins_manager,
|
||||
)
|
||||
.await
|
||||
{
|
||||
@@ -155,6 +166,7 @@ impl AppsRequestProcessor {
|
||||
config: Config,
|
||||
environment_manager: Arc<EnvironmentManager>,
|
||||
mcp_manager: Arc<McpManager>,
|
||||
plugins_manager: Arc<PluginsManager>,
|
||||
) -> Result<(AppsListResponse, bool), JSONRPCErrorError> {
|
||||
let AppsListParams {
|
||||
cursor,
|
||||
@@ -170,9 +182,13 @@ impl AppsRequestProcessor {
|
||||
None => 0,
|
||||
};
|
||||
|
||||
let plugin_apps = plugins_manager
|
||||
.plugins_for_config(&config.plugins_config_input())
|
||||
.await
|
||||
.effective_apps();
|
||||
let (mut accessible_connectors, mut all_connectors) = tokio::join!(
|
||||
connectors::list_cached_accessible_connectors_from_mcp_tools(&config),
|
||||
connectors::list_cached_all_connectors(&config)
|
||||
connectors::list_cached_all_connectors(&config, &plugin_apps)
|
||||
);
|
||||
let cached_all_connectors = all_connectors.clone();
|
||||
|
||||
@@ -193,10 +209,15 @@ impl AppsRequestProcessor {
|
||||
});
|
||||
|
||||
let all_config = config.clone();
|
||||
let all_plugin_apps = plugin_apps.clone();
|
||||
tokio::spawn(async move {
|
||||
let result = connectors::list_all_connectors_with_options(&all_config, force_refetch)
|
||||
.await
|
||||
.map_err(|err| format!("failed to list apps: {err}"));
|
||||
let result = connectors::list_all_connectors_with_options(
|
||||
&all_config,
|
||||
force_refetch,
|
||||
&all_plugin_apps,
|
||||
)
|
||||
.await
|
||||
.map_err(|err| format!("failed to list apps: {err}"));
|
||||
let _ = tx.send(AppListLoadResult::Directory(result));
|
||||
});
|
||||
|
||||
|
||||
@@ -1578,7 +1578,7 @@ impl PluginRequestProcessor {
|
||||
.as_ref()
|
||||
.map(plugin_app_category_by_id_from_value)
|
||||
.unwrap_or_default();
|
||||
let all_connectors = connectors::list_cached_all_connectors(&config)
|
||||
let all_connectors = connectors::list_cached_all_connectors(&config, &[])
|
||||
.await
|
||||
.unwrap_or_default();
|
||||
connectors::connectors_for_plugin_apps(all_connectors, &plugin_apps)
|
||||
@@ -1630,7 +1630,7 @@ impl PluginRequestProcessor {
|
||||
|
||||
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),
|
||||
connectors::list_all_connectors_with_options(config, /*force_refetch*/ false, &[]),
|
||||
connectors::list_accessible_connectors_from_mcp_tools_with_mcp_manager(
|
||||
config,
|
||||
/*force_refetch*/ true,
|
||||
@@ -1646,7 +1646,7 @@ impl PluginRequestProcessor {
|
||||
plugin = plugin_id,
|
||||
"failed to load app metadata after plugin install: {err:#}"
|
||||
);
|
||||
connectors::list_cached_all_connectors(config)
|
||||
connectors::list_cached_all_connectors(config, &[])
|
||||
.await
|
||||
.unwrap_or_default()
|
||||
}
|
||||
@@ -1910,16 +1910,21 @@ async fn load_plugin_app_summaries(
|
||||
return Vec::new();
|
||||
}
|
||||
|
||||
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 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);
|
||||
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use std::borrow::Cow;
|
||||
use std::collections::HashMap;
|
||||
use std::path::Path;
|
||||
use std::sync::Arc;
|
||||
use std::sync::Mutex as StdMutex;
|
||||
use std::time::Duration;
|
||||
@@ -215,6 +216,50 @@ async fn list_apps_returns_empty_when_workspace_codex_plugins_disabled() -> Resu
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn list_apps_includes_plugin_apps_for_chatgpt_auth() -> Result<()> {
|
||||
let (server_url, server_handle) =
|
||||
start_apps_server_with_delays(Vec::new(), Vec::new(), Duration::ZERO, Duration::ZERO)
|
||||
.await?;
|
||||
|
||||
let codex_home = TempDir::new()?;
|
||||
write_connectors_and_plugins_config(codex_home.path(), &server_url)?;
|
||||
write_plugin_app_fixture(codex_home.path(), "sample", "connector_sample")?;
|
||||
write_chatgpt_auth(
|
||||
codex_home.path(),
|
||||
ChatGptAuthFixture::new("chatgpt-token")
|
||||
.account_id("account-123")
|
||||
.chatgpt_user_id("user-plugin-apps")
|
||||
.chatgpt_account_id("account-123"),
|
||||
AuthCredentialsStoreMode::File,
|
||||
)?;
|
||||
|
||||
let mut mcp = TestAppServer::new(codex_home.path()).await?;
|
||||
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let request_id = mcp
|
||||
.send_apps_list_request(AppsListParams {
|
||||
limit: None,
|
||||
cursor: None,
|
||||
thread_id: None,
|
||||
force_refetch: false,
|
||||
})
|
||||
.await?;
|
||||
let response: JSONRPCResponse = timeout(
|
||||
DEFAULT_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
|
||||
)
|
||||
.await??;
|
||||
let AppsListResponse { data, next_cursor } = to_response(response)?;
|
||||
|
||||
assert!(data.iter().any(|app| app.id == "connector_sample"));
|
||||
assert!(next_cursor.is_none());
|
||||
|
||||
server_handle.abort();
|
||||
let _ = server_handle.await;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn list_apps_uses_thread_feature_flag_when_thread_id_is_provided() -> Result<()> {
|
||||
let connectors = vec![AppInfo {
|
||||
@@ -1623,3 +1668,45 @@ connectors = true
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
fn write_connectors_and_plugins_config(codex_home: &Path, base_url: &str) -> std::io::Result<()> {
|
||||
let config_toml = codex_home.join("config.toml");
|
||||
std::fs::write(
|
||||
config_toml,
|
||||
format!(
|
||||
r#"
|
||||
chatgpt_base_url = "{base_url}"
|
||||
mcp_oauth_credentials_store = "file"
|
||||
|
||||
[features]
|
||||
connectors = true
|
||||
plugins = true
|
||||
|
||||
[plugins."sample@test"]
|
||||
enabled = true
|
||||
"#
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
fn write_plugin_app_fixture(codex_home: &Path, plugin_name: &str, app_id: &str) -> Result<()> {
|
||||
let plugin_root = codex_home
|
||||
.join("plugins/cache")
|
||||
.join("test")
|
||||
.join(plugin_name)
|
||||
.join("local");
|
||||
std::fs::create_dir_all(plugin_root.join(".codex-plugin"))?;
|
||||
std::fs::write(
|
||||
plugin_root.join(".codex-plugin/plugin.json"),
|
||||
format!(r#"{{"name":"{plugin_name}"}}"#),
|
||||
)?;
|
||||
std::fs::write(
|
||||
plugin_root.join(".app.json"),
|
||||
serde_json::to_vec_pretty(&json!({
|
||||
"apps": {
|
||||
plugin_name: { "id": app_id }
|
||||
}
|
||||
}))?,
|
||||
)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -13,7 +13,6 @@ clap = { workspace = true, features = ["derive"] }
|
||||
codex-app-server-protocol = { workspace = true }
|
||||
codex-connectors = { workspace = true }
|
||||
codex-core = { workspace = true }
|
||||
codex-core-plugins = { workspace = true }
|
||||
codex-git-utils = { workspace = true }
|
||||
codex-login = { workspace = true }
|
||||
codex-model-provider = { workspace = true }
|
||||
|
||||
@@ -19,7 +19,6 @@ pub use codex_core::connectors::list_accessible_connectors_from_mcp_tools_with_o
|
||||
pub use codex_core::connectors::list_accessible_connectors_from_mcp_tools_with_options_and_status;
|
||||
pub use codex_core::connectors::list_cached_accessible_connectors_from_mcp_tools;
|
||||
pub use codex_core::connectors::with_app_enabled_state;
|
||||
use codex_core_plugins::PluginsManager;
|
||||
use codex_login::AuthManager;
|
||||
use codex_login::CodexAuth;
|
||||
use codex_login::default_client::originator;
|
||||
@@ -69,10 +68,13 @@ pub async fn list_connectors(config: &Config) -> anyhow::Result<Vec<AppInfo>> {
|
||||
}
|
||||
|
||||
pub async fn list_all_connectors(config: &Config) -> anyhow::Result<Vec<AppInfo>> {
|
||||
list_all_connectors_with_options(config, /*force_refetch*/ false).await
|
||||
list_all_connectors_with_options(config, /*force_refetch*/ false, &[]).await
|
||||
}
|
||||
|
||||
pub async fn list_cached_all_connectors(config: &Config) -> Option<Vec<AppInfo>> {
|
||||
pub async fn list_cached_all_connectors(
|
||||
config: &Config,
|
||||
plugin_apps: &[AppConnectorId],
|
||||
) -> Option<Vec<AppInfo>> {
|
||||
if !apps_enabled(config).await {
|
||||
return Some(Vec::new());
|
||||
}
|
||||
@@ -80,22 +82,13 @@ pub async fn list_cached_all_connectors(config: &Config) -> Option<Vec<AppInfo>>
|
||||
let auth = connector_auth(config).await.ok()?;
|
||||
let cache_context = connector_directory_cache_context(config, &auth);
|
||||
let connectors = codex_connectors::cached_directory_connectors(&cache_context)?;
|
||||
let connectors = merge_plugin_connectors(
|
||||
connectors,
|
||||
plugin_apps_for_config(config)
|
||||
.await
|
||||
.into_iter()
|
||||
.map(|connector_id| connector_id.0),
|
||||
);
|
||||
Some(filter_disallowed_connectors(
|
||||
connectors,
|
||||
originator().value.as_str(),
|
||||
))
|
||||
Some(merge_and_filter_plugin_connectors(connectors, plugin_apps))
|
||||
}
|
||||
|
||||
pub async fn list_all_connectors_with_options(
|
||||
config: &Config,
|
||||
force_refetch: bool,
|
||||
plugin_apps: &[AppConnectorId],
|
||||
) -> anyhow::Result<Vec<AppInfo>> {
|
||||
if !apps_enabled(config).await {
|
||||
return Ok(Vec::new());
|
||||
@@ -116,17 +109,7 @@ pub async fn list_all_connectors_with_options(
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
let connectors = merge_plugin_connectors(
|
||||
connectors,
|
||||
plugin_apps_for_config(config)
|
||||
.await
|
||||
.into_iter()
|
||||
.map(|connector_id| connector_id.0),
|
||||
);
|
||||
Ok(filter_disallowed_connectors(
|
||||
connectors,
|
||||
originator().value.as_str(),
|
||||
))
|
||||
Ok(merge_and_filter_plugin_connectors(connectors, plugin_apps))
|
||||
}
|
||||
|
||||
fn connector_directory_cache_context(
|
||||
@@ -144,12 +127,17 @@ fn connector_directory_cache_context(
|
||||
)
|
||||
}
|
||||
|
||||
async fn plugin_apps_for_config(config: &Config) -> Vec<AppConnectorId> {
|
||||
let plugins_input = config.plugins_config_input();
|
||||
PluginsManager::new(config.codex_home.to_path_buf())
|
||||
.plugins_for_config(&plugins_input)
|
||||
.await
|
||||
.effective_apps()
|
||||
fn merge_and_filter_plugin_connectors(
|
||||
connectors: Vec<AppInfo>,
|
||||
plugin_apps: &[AppConnectorId],
|
||||
) -> Vec<AppInfo> {
|
||||
let connectors = merge_plugin_connectors(
|
||||
connectors,
|
||||
plugin_apps
|
||||
.iter()
|
||||
.map(|connector_id| connector_id.0.clone()),
|
||||
);
|
||||
filter_disallowed_connectors(connectors, originator().value.as_str())
|
||||
}
|
||||
|
||||
pub fn connectors_for_plugin_apps(
|
||||
|
||||
Reference in New Issue
Block a user