mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
feat: load plugin apps (#13401)
load plugin-apps from `.app.json`. make apps runtime-mentionable iff `codex_apps` MCP actually exposes tools for that `connector_id`. if the app isn't available, it's filtered out of runtime connector set, so no tools are added and no app-mentions resolve. right now we don't have a clean cli-side error for an app not being installed. can look at this after. ### Tests Added tests, tested locally that using a plugin that bundles an app picks up the app.
This commit is contained in:
@@ -4891,10 +4891,15 @@ pub(crate) async fn run_turn(
|
||||
Ok(mcp_tools) => mcp_tools,
|
||||
Err(_) => return None,
|
||||
};
|
||||
connectors::with_app_enabled_state(
|
||||
let plugin_apps = sess
|
||||
.services
|
||||
.plugins_manager
|
||||
.plugins_for_config(&turn_context.config);
|
||||
let connectors = connectors::merge_plugin_apps_with_accessible(
|
||||
plugin_apps.effective_apps(),
|
||||
connectors::accessible_connectors_from_mcp_tools(&mcp_tools),
|
||||
&turn_context.config,
|
||||
)
|
||||
);
|
||||
connectors::with_app_enabled_state(connectors, &turn_context.config)
|
||||
} else {
|
||||
Vec::new()
|
||||
};
|
||||
@@ -5614,8 +5619,16 @@ async fn built_tools(
|
||||
effective_explicitly_enabled_connectors.extend(sess.get_connector_selection().await);
|
||||
|
||||
let connectors = if turn_context.features.enabled(Feature::Apps) {
|
||||
Some(connectors::with_app_enabled_state(
|
||||
let plugin_apps = sess
|
||||
.services
|
||||
.plugins_manager
|
||||
.plugins_for_config(&turn_context.config);
|
||||
let connectors = connectors::merge_plugin_apps_with_accessible(
|
||||
plugin_apps.effective_apps(),
|
||||
connectors::accessible_connectors_from_mcp_tools(&mcp_tools),
|
||||
);
|
||||
Some(connectors::with_app_enabled_state(
|
||||
connectors,
|
||||
&turn_context.config,
|
||||
))
|
||||
} else {
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
use std::env;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::LazyLock;
|
||||
@@ -29,6 +30,7 @@ use crate::mcp::auth::compute_auth_statuses;
|
||||
use crate::mcp::with_codex_apps_mcp;
|
||||
use crate::mcp_connection_manager::McpConnectionManager;
|
||||
use crate::mcp_connection_manager::codex_apps_tools_cache_key;
|
||||
use crate::plugins::AppConnectorId;
|
||||
use crate::token_data::TokenData;
|
||||
|
||||
pub const CONNECTORS_CACHE_TTL: Duration = Duration::from_secs(3600);
|
||||
@@ -353,6 +355,48 @@ pub fn merge_connectors(
|
||||
merged
|
||||
}
|
||||
|
||||
pub fn merge_plugin_apps(
|
||||
connectors: Vec<AppInfo>,
|
||||
plugin_apps: Vec<AppConnectorId>,
|
||||
) -> Vec<AppInfo> {
|
||||
let mut merged = connectors;
|
||||
let mut connector_ids = merged
|
||||
.iter()
|
||||
.map(|connector| connector.id.clone())
|
||||
.collect::<HashSet<_>>();
|
||||
|
||||
for connector_id in plugin_apps {
|
||||
if connector_ids.insert(connector_id.0.clone()) {
|
||||
merged.push(plugin_app_to_app_info(connector_id));
|
||||
}
|
||||
}
|
||||
|
||||
merged.sort_by(|left, right| {
|
||||
right
|
||||
.is_accessible
|
||||
.cmp(&left.is_accessible)
|
||||
.then_with(|| left.name.cmp(&right.name))
|
||||
.then_with(|| left.id.cmp(&right.id))
|
||||
});
|
||||
merged
|
||||
}
|
||||
|
||||
pub fn merge_plugin_apps_with_accessible(
|
||||
plugin_apps: Vec<AppConnectorId>,
|
||||
accessible_connectors: Vec<AppInfo>,
|
||||
) -> Vec<AppInfo> {
|
||||
let accessible_connector_ids: HashSet<&str> = accessible_connectors
|
||||
.iter()
|
||||
.map(|connector| connector.id.as_str())
|
||||
.collect();
|
||||
let plugin_connectors = plugin_apps
|
||||
.into_iter()
|
||||
.filter(|connector_id| accessible_connector_ids.contains(connector_id.0.as_str()))
|
||||
.map(plugin_app_to_app_info)
|
||||
.collect::<Vec<_>>();
|
||||
merge_connectors(plugin_connectors, accessible_connectors)
|
||||
}
|
||||
|
||||
pub fn with_app_enabled_state(mut connectors: Vec<AppInfo>, config: &Config) -> Vec<AppInfo> {
|
||||
let apps_config = read_apps_config(config);
|
||||
if let Some(apps_config) = apps_config.as_ref() {
|
||||
@@ -575,6 +619,28 @@ where
|
||||
accessible
|
||||
}
|
||||
|
||||
fn plugin_app_to_app_info(connector_id: AppConnectorId) -> AppInfo {
|
||||
// Leave the placeholder name as the connector id so merge_connectors() can
|
||||
// replace it with canonical app metadata from directory fetches or
|
||||
// connector_name values from codex_apps tool discovery.
|
||||
let connector_id = connector_id.0;
|
||||
let name = connector_id.clone();
|
||||
AppInfo {
|
||||
id: connector_id.clone(),
|
||||
name: name.clone(),
|
||||
description: None,
|
||||
logo_url: None,
|
||||
logo_url_dark: None,
|
||||
distribution_channel: None,
|
||||
branding: None,
|
||||
app_metadata: None,
|
||||
labels: None,
|
||||
install_url: Some(connector_install_url(&name, &connector_id)),
|
||||
is_accessible: false,
|
||||
is_enabled: true,
|
||||
}
|
||||
}
|
||||
|
||||
fn normalize_connector_value(value: Option<&str>) -> Option<String> {
|
||||
value
|
||||
.map(str::trim)
|
||||
@@ -647,6 +713,46 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn merge_connectors_replaces_plugin_placeholder_name_with_accessible_name() {
|
||||
let plugin = plugin_app_to_app_info(AppConnectorId("calendar".to_string()));
|
||||
let accessible = AppInfo {
|
||||
id: "calendar".to_string(),
|
||||
name: "Google Calendar".to_string(),
|
||||
description: Some("Plan events".to_string()),
|
||||
logo_url: Some("https://example.com/logo.png".to_string()),
|
||||
logo_url_dark: Some("https://example.com/logo-dark.png".to_string()),
|
||||
distribution_channel: Some("workspace".to_string()),
|
||||
branding: None,
|
||||
app_metadata: None,
|
||||
labels: None,
|
||||
install_url: None,
|
||||
is_accessible: true,
|
||||
is_enabled: true,
|
||||
};
|
||||
|
||||
let merged = merge_connectors(vec![plugin], vec![accessible]);
|
||||
|
||||
assert_eq!(
|
||||
merged,
|
||||
vec![AppInfo {
|
||||
id: "calendar".to_string(),
|
||||
name: "Google Calendar".to_string(),
|
||||
description: Some("Plan events".to_string()),
|
||||
logo_url: Some("https://example.com/logo.png".to_string()),
|
||||
logo_url_dark: Some("https://example.com/logo-dark.png".to_string()),
|
||||
distribution_channel: Some("workspace".to_string()),
|
||||
branding: None,
|
||||
app_metadata: None,
|
||||
labels: None,
|
||||
install_url: Some(connector_install_url("calendar", "calendar")),
|
||||
is_accessible: true,
|
||||
is_enabled: true,
|
||||
}]
|
||||
);
|
||||
assert_eq!(connector_mention_slug(&merged[0]), "google-calendar");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn app_tool_policy_uses_global_defaults_for_destructive_hints() {
|
||||
let apps_config = AppsConfigToml {
|
||||
|
||||
@@ -33,6 +33,10 @@ use tracing::warn;
|
||||
|
||||
const DEFAULT_SKILLS_DIR_NAME: &str = "skills";
|
||||
const DEFAULT_MCP_CONFIG_FILE: &str = ".mcp.json";
|
||||
const DEFAULT_APP_CONFIG_FILE: &str = ".app.json";
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
|
||||
pub struct AppConnectorId(pub String);
|
||||
|
||||
#[derive(Debug, Clone, PartialEq)]
|
||||
pub struct LoadedPlugin {
|
||||
@@ -42,6 +46,7 @@ pub struct LoadedPlugin {
|
||||
pub enabled: bool,
|
||||
pub skill_roots: Vec<PathBuf>,
|
||||
pub mcp_servers: HashMap<String, McpServerConfig>,
|
||||
pub apps: Vec<AppConnectorId>,
|
||||
pub error: Option<String>,
|
||||
}
|
||||
|
||||
@@ -80,6 +85,21 @@ impl PluginLoadOutcome {
|
||||
}
|
||||
mcp_servers
|
||||
}
|
||||
|
||||
pub fn effective_apps(&self) -> Vec<AppConnectorId> {
|
||||
let mut apps = Vec::new();
|
||||
let mut seen_connector_ids = std::collections::HashSet::new();
|
||||
|
||||
for plugin in self.plugins.iter().filter(|plugin| plugin.is_active()) {
|
||||
for connector_id in &plugin.apps {
|
||||
if seen_connector_ids.insert(connector_id.clone()) {
|
||||
apps.push(connector_id.clone());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
apps
|
||||
}
|
||||
}
|
||||
|
||||
pub struct PluginsManager {
|
||||
@@ -227,6 +247,18 @@ struct PluginMcpFile {
|
||||
mcp_servers: HashMap<String, JsonValue>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Default, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
struct PluginAppFile {
|
||||
#[serde(default)]
|
||||
apps: HashMap<String, PluginAppConfig>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Default, Deserialize)]
|
||||
struct PluginAppConfig {
|
||||
id: String,
|
||||
}
|
||||
|
||||
pub(crate) fn load_plugins_from_layer_stack(
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
store: &PluginStore,
|
||||
@@ -299,6 +331,7 @@ fn load_plugin(config_name: String, plugin: &PluginConfig, store: &PluginStore)
|
||||
enabled: plugin.enabled,
|
||||
skill_roots: Vec::new(),
|
||||
mcp_servers: HashMap::new(),
|
||||
apps: Vec::new(),
|
||||
error: None,
|
||||
};
|
||||
|
||||
@@ -341,6 +374,10 @@ fn load_plugin(config_name: String, plugin: &PluginConfig, store: &PluginStore)
|
||||
}
|
||||
}
|
||||
loaded_plugin.mcp_servers = mcp_servers;
|
||||
loaded_plugin.apps = load_apps_from_file(
|
||||
plugin_root.as_path(),
|
||||
&plugin_root.as_path().join(DEFAULT_APP_CONFIG_FILE),
|
||||
);
|
||||
loaded_plugin
|
||||
}
|
||||
|
||||
@@ -364,6 +401,39 @@ fn default_mcp_config_paths(plugin_root: &Path) -> Vec<PathBuf> {
|
||||
paths
|
||||
}
|
||||
|
||||
fn load_apps_from_file(plugin_root: &Path, app_config_path: &Path) -> Vec<AppConnectorId> {
|
||||
let Ok(contents) = fs::read_to_string(app_config_path) else {
|
||||
return Vec::new();
|
||||
};
|
||||
let parsed = match serde_json::from_str::<PluginAppFile>(&contents) {
|
||||
Ok(parsed) => parsed,
|
||||
Err(err) => {
|
||||
warn!(
|
||||
path = %app_config_path.display(),
|
||||
"failed to parse plugin app config: {err}"
|
||||
);
|
||||
return Vec::new();
|
||||
}
|
||||
};
|
||||
|
||||
let mut apps: Vec<PluginAppConfig> = parsed.apps.into_values().collect();
|
||||
apps.sort_unstable_by(|left, right| left.id.cmp(&right.id));
|
||||
|
||||
apps.into_iter()
|
||||
.filter_map(|app| {
|
||||
if app.id.trim().is_empty() {
|
||||
warn!(
|
||||
plugin = %plugin_root.display(),
|
||||
"plugin app config is missing an app id"
|
||||
);
|
||||
None
|
||||
} else {
|
||||
Some(AppConnectorId(app.id))
|
||||
}
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn load_mcp_servers_from_file(plugin_root: &Path, mcp_config_path: &Path) -> PluginMcpDiscovery {
|
||||
let Ok(contents) = fs::read_to_string(mcp_config_path) else {
|
||||
return PluginMcpDiscovery::default();
|
||||
@@ -550,6 +620,16 @@ mod tests {
|
||||
}
|
||||
}"#,
|
||||
);
|
||||
write_file(
|
||||
&plugin_root.join(".app.json"),
|
||||
r#"{
|
||||
"apps": {
|
||||
"example": {
|
||||
"id": "connector_example"
|
||||
}
|
||||
}
|
||||
}"#,
|
||||
);
|
||||
|
||||
let outcome =
|
||||
load_plugins_from_config(&plugin_config_toml(true, true), codex_home.path()).await;
|
||||
@@ -582,6 +662,7 @@ mod tests {
|
||||
oauth_resource: None,
|
||||
},
|
||||
)]),
|
||||
apps: vec![AppConnectorId("connector_example".to_string())],
|
||||
error: None,
|
||||
}]
|
||||
);
|
||||
@@ -590,6 +671,10 @@ mod tests {
|
||||
vec![plugin_root.join("skills")]
|
||||
);
|
||||
assert_eq!(outcome.effective_mcp_servers().len(), 1);
|
||||
assert_eq!(
|
||||
outcome.effective_apps(),
|
||||
vec![AppConnectorId("connector_example".to_string())]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -628,6 +713,7 @@ mod tests {
|
||||
enabled: false,
|
||||
skill_roots: Vec::new(),
|
||||
mcp_servers: HashMap::new(),
|
||||
apps: Vec::new(),
|
||||
error: None,
|
||||
}]
|
||||
);
|
||||
@@ -635,6 +721,80 @@ mod tests {
|
||||
assert!(outcome.effective_mcp_servers().is_empty());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn effective_apps_dedupes_connector_ids_across_plugins() {
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
let plugin_a_root = codex_home
|
||||
.path()
|
||||
.join("plugins/cache")
|
||||
.join("test/plugin-a/local");
|
||||
let plugin_b_root = codex_home
|
||||
.path()
|
||||
.join("plugins/cache")
|
||||
.join("test/plugin-b/local");
|
||||
|
||||
write_file(
|
||||
&plugin_a_root.join(".codex-plugin/plugin.json"),
|
||||
r#"{"name":"plugin-a"}"#,
|
||||
);
|
||||
write_file(
|
||||
&plugin_a_root.join(".app.json"),
|
||||
r#"{
|
||||
"apps": {
|
||||
"example": {
|
||||
"id": "connector_example"
|
||||
}
|
||||
}
|
||||
}"#,
|
||||
);
|
||||
write_file(
|
||||
&plugin_b_root.join(".codex-plugin/plugin.json"),
|
||||
r#"{"name":"plugin-b"}"#,
|
||||
);
|
||||
write_file(
|
||||
&plugin_b_root.join(".app.json"),
|
||||
r#"{
|
||||
"apps": {
|
||||
"chat": {
|
||||
"id": "connector_example"
|
||||
},
|
||||
"gmail": {
|
||||
"id": "connector_gmail"
|
||||
}
|
||||
}
|
||||
}"#,
|
||||
);
|
||||
|
||||
let mut root = toml::map::Map::new();
|
||||
let mut features = toml::map::Map::new();
|
||||
features.insert("plugins".to_string(), Value::Boolean(true));
|
||||
root.insert("features".to_string(), Value::Table(features));
|
||||
|
||||
let mut plugins = toml::map::Map::new();
|
||||
|
||||
let mut plugin_a = toml::map::Map::new();
|
||||
plugin_a.insert("enabled".to_string(), Value::Boolean(true));
|
||||
plugins.insert("plugin-a@test".to_string(), Value::Table(plugin_a));
|
||||
|
||||
let mut plugin_b = toml::map::Map::new();
|
||||
plugin_b.insert("enabled".to_string(), Value::Boolean(true));
|
||||
plugins.insert("plugin-b@test".to_string(), Value::Table(plugin_b));
|
||||
|
||||
root.insert("plugins".to_string(), Value::Table(plugins));
|
||||
let config_toml =
|
||||
toml::to_string(&Value::Table(root)).expect("plugin test config should serialize");
|
||||
|
||||
let outcome = load_plugins_from_config(&config_toml, codex_home.path()).await;
|
||||
|
||||
assert_eq!(
|
||||
outcome.effective_apps(),
|
||||
vec![
|
||||
AppConnectorId("connector_example".to_string()),
|
||||
AppConnectorId("connector_gmail".to_string()),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn plugin_namespace_for_skill_path_uses_manifest_name() {
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
|
||||
@@ -2,6 +2,7 @@ mod manager;
|
||||
mod manifest;
|
||||
mod store;
|
||||
|
||||
pub use manager::AppConnectorId;
|
||||
pub use manager::LoadedPlugin;
|
||||
pub use manager::PluginInstallError;
|
||||
pub use manager::PluginLoadOutcome;
|
||||
|
||||
Reference in New Issue
Block a user