mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Skip plugin MCP OAuth for matching app routes (#27461)
## Context This is PR5 in the plugin auth-routing stack. Earlier PRs make plugin surface projection auth-aware, narrow App/MCP conflicts by App declaration name, and keep connector listings auth-aware. This PR applies the same name-based App/MCP conflict rule into plugin MCP loading, so install-time MCP OAuth and plugin detail metadata both reflect the MCPs available for the current auth route. ## 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 - Make `load_plugin_mcp_servers` auth-aware and let it load App declarations before filtering same-name MCP servers for Codex-backend auth. - Use that filtered MCP list for both install-time MCP OAuth and marketplace plugin detail metadata. - Preserve API-key/direct auth behavior so plugin MCP servers remain visible and can still start OAuth. ## Validation ```bash cargo fmt --all cargo test -p codex-core-plugins read_plugin_for_config_filters_mcp_servers_for_codex_backend_auth cargo check -p codex-core-plugins -p codex-app-server git diff --check git diff --cached --check ```
This commit is contained in:
committed by
GitHub
Unverified
parent
cededa26c5
commit
e5253b97fb
@@ -10,6 +10,7 @@ use crate::remote::REMOTE_GLOBAL_MARKETPLACE_NAME;
|
||||
use crate::remote::RemoteInstalledPlugin;
|
||||
use crate::store::PluginStore;
|
||||
use crate::store::plugin_version_for_source;
|
||||
use codex_app_server_protocol::AuthMode;
|
||||
use codex_config::ConfigLayerStack;
|
||||
use codex_config::HooksFile;
|
||||
use codex_config::types::McpServerConfig;
|
||||
@@ -1079,7 +1080,29 @@ pub async fn plugin_telemetry_metadata_from_root(
|
||||
}
|
||||
}
|
||||
|
||||
pub async fn load_plugin_mcp_servers(plugin_root: &Path) -> HashMap<String, McpServerConfig> {
|
||||
pub async fn load_plugin_mcp_servers(
|
||||
plugin_root: &Path,
|
||||
auth_mode: Option<AuthMode>,
|
||||
) -> HashMap<String, McpServerConfig> {
|
||||
let mut mcp_servers = load_declared_plugin_mcp_servers(plugin_root).await;
|
||||
if !auth_mode.is_some_and(AuthMode::uses_codex_backend) || mcp_servers.is_empty() {
|
||||
return mcp_servers;
|
||||
}
|
||||
|
||||
let app_declarations = load_plugin_apps(plugin_root).await;
|
||||
if app_declarations.is_empty() {
|
||||
return mcp_servers;
|
||||
}
|
||||
|
||||
let app_declaration_names = app_declarations
|
||||
.iter()
|
||||
.map(|app| app.name.as_str())
|
||||
.collect::<HashSet<_>>();
|
||||
mcp_servers.retain(|name, _| !app_declaration_names.contains(name.as_str()));
|
||||
mcp_servers
|
||||
}
|
||||
|
||||
async fn load_declared_plugin_mcp_servers(plugin_root: &Path) -> HashMap<String, McpServerConfig> {
|
||||
let Some(manifest) = load_plugin_manifest(plugin_root) else {
|
||||
return HashMap::new();
|
||||
};
|
||||
|
||||
@@ -1330,7 +1330,7 @@ impl PluginsManager {
|
||||
app_category_by_id.insert(app.connector_id.0.clone(), category.clone());
|
||||
}
|
||||
}
|
||||
let mut mcp_server_names = load_plugin_mcp_servers(source_path.as_path())
|
||||
let mut mcp_server_names = load_plugin_mcp_servers(source_path.as_path(), self.auth_mode())
|
||||
.await
|
||||
.into_keys()
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
@@ -2425,6 +2425,82 @@ enabled = true
|
||||
assert!(matches!(err, MarketplaceError::PluginsDisabled));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn read_plugin_for_config_filters_mcp_servers_for_codex_backend_auth() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let repo_root = tmp.path().join("repo");
|
||||
fs::create_dir_all(repo_root.join(".git")).unwrap();
|
||||
fs::create_dir_all(repo_root.join(".agents/plugins")).unwrap();
|
||||
write_file(
|
||||
&repo_root.join(".agents/plugins/marketplace.json"),
|
||||
r#"{
|
||||
"name": "debug",
|
||||
"plugins": [
|
||||
{
|
||||
"name": "sample-plugin",
|
||||
"source": {
|
||||
"source": "local",
|
||||
"path": "./sample-plugin"
|
||||
}
|
||||
}
|
||||
]
|
||||
}"#,
|
||||
);
|
||||
write_file(
|
||||
&repo_root.join("sample-plugin/.codex-plugin/plugin.json"),
|
||||
r#"{"name":"sample-plugin"}"#,
|
||||
);
|
||||
write_file(
|
||||
&repo_root.join("sample-plugin/.app.json"),
|
||||
r#"{"apps":{"sample-mcp":{"id":"connector_sample"}}}"#,
|
||||
);
|
||||
write_file(
|
||||
&repo_root.join("sample-plugin/.mcp.json"),
|
||||
r#"{"mcpServers":{"other-mcp":{"command":"other-mcp"},"sample-mcp":{"command":"sample-mcp"}}}"#,
|
||||
);
|
||||
write_file(
|
||||
&tmp.path().join(CONFIG_TOML_FILE),
|
||||
r#"[features]
|
||||
plugins = true
|
||||
"#,
|
||||
);
|
||||
|
||||
let config = load_config(tmp.path(), &repo_root).await;
|
||||
let request = PluginReadRequest {
|
||||
plugin_name: "sample-plugin".to_string(),
|
||||
marketplace_path: AbsolutePathBuf::try_from(
|
||||
repo_root.join(".agents/plugins/marketplace.json"),
|
||||
)
|
||||
.unwrap(),
|
||||
};
|
||||
|
||||
let chatgpt_outcome = PluginsManager::new_with_options(
|
||||
tmp.path().to_path_buf(),
|
||||
Some(Product::Codex),
|
||||
Some(AuthMode::Chatgpt),
|
||||
)
|
||||
.read_plugin_for_config(&config, &request)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
chatgpt_outcome.plugin.mcp_server_names,
|
||||
vec!["other-mcp".to_string()]
|
||||
);
|
||||
|
||||
let api_key_outcome = PluginsManager::new_with_options(
|
||||
tmp.path().to_path_buf(),
|
||||
Some(Product::Codex),
|
||||
Some(AuthMode::ApiKey),
|
||||
)
|
||||
.read_plugin_for_config(&config, &request)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
api_key_outcome.plugin.mcp_server_names,
|
||||
vec!["other-mcp".to_string(), "sample-mcp".to_string()]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn read_plugin_for_config_uses_user_layer_skill_settings_only() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
|
||||
Reference in New Issue
Block a user