agent/codex
2
0
Fork 0
mirror of https://github.com/pchuan98/codex.git synced 2026-07-01 00:31:56 +08:00
Code Issues Packages Projects Releases Wiki Activity

Extract shared plugin MCP config parsing (#27863)

Browse Source 
## Why

We want a thread-selected plugin to eventually expose stdio MCP servers
that run on the executor owning that plugin.

The existing plugin MCP parser lived inside `core-plugins` and was
coupled to the host filesystem loader. Reusing it from an executor
provider would either duplicate MCP normalization or make the plugin
package layer own MCP runtime semantics. This PR creates the shared
MCP-owned boundary first.

In simple terms:

```text
plugin .mcp.json
        |
        v
shared parser in codex-mcp
        |
        +-- Declared placement: preserve current local-plugin behavior
        |
        +-- Environment placement: produce config bound to one executor
```

This builds on the authority-bound plugin descriptors from #27692. It
intentionally does not discover, register, or launch executor MCP
servers yet.

## What changed

- Moved plugin MCP file parsing and normalization from `core-plugins`
into `codex-mcp`.
- Kept support for both existing file shapes: a top-level server map and
an object containing `mcpServers`.
- Kept per-server failure isolation: one invalid server does not discard
valid siblings, while malformed top-level JSON still fails the whole
file.
- Updated the existing local plugin loader to use `Declared` placement,
preserving its current transport, OAuth, relative `cwd`, and error
behavior.
- Added `Environment` placement for the next stacked PR:
- the selected environment ID overrides anything declared by the plugin;
  - missing stdio `cwd` defaults to the plugin root;
- relative `cwd` is resolved beneath the plugin root and cannot traverse
outside it;
- bare or source-less environment-variable references resolve on a
non-local executor;
- explicit orchestrator environment-variable forwarding is rejected for
executor-owned plugins.

## User impact

None in this PR. Existing local plugin MCP loading follows the same
behavior through the shared parser. The executor placement mode is not
connected to thread startup until the follow-up registration PR.

## Assumptions

- A selected capability root's environment is authoritative. A plugin
cannot redirect its stdio process to the orchestrator or another
executor.
- Relative working directories belong under the plugin package root.
Explicit absolute working directories remain valid within the owning
environment.
- For a non-local executor, unqualified environment-variable names refer
to that executor. Reading an orchestrator variable requires an explicit
contract and is rejected for now.
- Parsing only produces normalized `McpServerConfig` values. Process
startup remains owned by the existing MCP runtime and connection
manager.

## Follow-ups

1. Add the executor MCP provider and catalog registration: read the
selected plugin's MCP config through the same executor filesystem,
support stdio only, freeze the result per active thread, apply managed
policy, and resolve name collisions as discovered plugin < selected
plugin < explicit config.
2. Install that provider in app-server and add an end-to-end test
proving `thread/start.selectedCapabilityRoots` launches and calls the
MCP tool on the selected executor, preserves the frozen registration
across refresh, and does not expose it to an unselected thread.
3. After the initial executor-stdio vertical, define
resume/fork/environment-replacement semantics, executor HTTP placement,
warning delivery, common MCP tool-context bounds, and move remaining MCP
source composition above core.

## Verification

- `cargo check -p codex-mcp -p codex-core-plugins --tests`
- `just bazel-lock-check`
- Added focused parser coverage for legacy local normalization, executor
authority, working-directory handling, and environment-variable
sourcing.
This commit is contained in:
jif
2026-06-12 15:10:05 +02:00
committed by GitHub
parent 267eacfca2
commit 17b9f4843e
7 changed files with 562 additions and 186 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core-plugins/src/loader.rs
+19 -110
View File
@@ -22,6 +22,8 @@ use codex_core_skills::config_rules::skill_config_rules_from_stack;
use codex_core_skills::loader::SkillRoot;
use codex_core_skills::loader::load_skills_from_roots;
use codex_exec_server::LOCAL_FS;
use codex_mcp::PluginMcpServerPlacement;
use codex_mcp::parse_plugin_mcp_config;
use codex_plugin::AppConnectorId;
use codex_plugin::LoadedPlugin;
use codex_plugin::PluginCapabilitySummary;
@@ -36,7 +38,6 @@ use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_plugins::find_plugin_manifest_path;
use indexmap::IndexMap;
use serde::Deserialize;
use serde_json::Map as JsonMap;
use serde_json::Value as JsonValue;
use std::collections::HashMap;
use std::collections::HashSet;
@@ -97,28 +98,6 @@ pub fn log_plugin_load_errors(outcome: &PluginLoadOutcome<McpServerConfig>) {
}
}
#[derive(Debug, Default, Deserialize)]
#[serde(rename_all = "camelCase")]
struct PluginMcpServersFile {
mcp_servers: HashMap<String, JsonValue>,
}
#[derive(Debug, Deserialize)]
#[serde(untagged)]
enum PluginMcpFile {
McpServersObject(PluginMcpServersFile),
ServerMap(HashMap<String, JsonValue>),
}
impl PluginMcpFile {
fn into_mcp_servers(self) -> HashMap<String, JsonValue> {
match self {
Self::McpServersObject(file) => file.mcp_servers,
Self::ServerMap(mcp_servers) => mcp_servers,
}
}
}
#[derive(Debug, Default, Deserialize)]
#[serde(rename_all = "camelCase")]
struct PluginAppFile {
@@ -1156,99 +1135,29 @@ async fn load_mcp_servers_from_file(
let Ok(contents) = tokio::fs::read_to_string(mcp_config_path.as_path()).await else {
return PluginMcpDiscovery::default();
};
let parsed = match serde_json::from_str::<PluginMcpFile>(&contents) {
Ok(parsed) => parsed,
Err(err) => {
warn!(
path = %mcp_config_path.display(),
"failed to parse plugin MCP config: {err}"
);
return PluginMcpDiscovery::default();
}
};
normalize_plugin_mcp_servers(
plugin_root,
parsed.into_mcp_servers(),
mcp_config_path.to_string_lossy().as_ref(),
)
}
fn normalize_plugin_mcp_servers(
plugin_root: &Path,
plugin_mcp_servers: HashMap<String, JsonValue>,
source: &str,
) -> PluginMcpDiscovery {
let mut mcp_servers = HashMap::new();
for (name, config_value) in plugin_mcp_servers {
let normalized = normalize_plugin_mcp_server_value(plugin_root, config_value);
match serde_json::from_value::<McpServerConfig>(JsonValue::Object(normalized)) {
Ok(config) => {
mcp_servers.insert(name, config);
}
let parsed =
match parse_plugin_mcp_config(plugin_root, &contents, PluginMcpServerPlacement::Declared) {
Ok(parsed) => parsed,
Err(err) => {
warn!(
plugin = %plugin_root.display(),
server = name,
"failed to parse plugin MCP server from {source}: {err}"
path = %mcp_config_path.display(),
"failed to parse plugin MCP config: {err}"
);
return PluginMcpDiscovery::default();
}
}
}
PluginMcpDiscovery { mcp_servers }
}
fn normalize_plugin_mcp_server_value(
plugin_root: &Path,
value: JsonValue,
) -> JsonMap<String, JsonValue> {
let mut object = match value {
JsonValue::Object(object) => object,
_ => return JsonMap::new(),
};
if let Some(JsonValue::String(transport_type)) = object.remove("type") {
match transport_type.as_str() {
"http" | "streamable_http" | "streamable-http" => {}
"stdio" => {}
other => {
warn!(
plugin = %plugin_root.display(),
transport = other,
"plugin MCP server uses an unknown transport type"
);
}
}
}
if let Some(JsonValue::Object(mut oauth)) = object.remove("oauth") {
if oauth.remove("callbackPort").is_some() {
warn!(
plugin = %plugin_root.display(),
"plugin MCP server OAuth callbackPort is ignored; Codex uses global MCP OAuth callback settings"
);
}
if let Some(client_id) = oauth.remove("clientId") {
oauth.entry("client_id".to_string()).or_insert(client_id);
}
if !oauth.is_empty() {
object.insert("oauth".to_string(), JsonValue::Object(oauth));
}
}
if let Some(JsonValue::String(cwd)) = object.get("cwd")
&& !Path::new(cwd).is_absolute()
{
object.insert(
"cwd".to_string(),
JsonValue::String(plugin_root.join(cwd).display().to_string()),
};
for error in parsed.errors {
warn!(
plugin = %plugin_root.display(),
server = error.name,
path = %mcp_config_path.display(),
error = error.message,
"failed to parse plugin MCP server"
);
}
object
PluginMcpDiscovery {
mcp_servers: parsed.servers.into_iter().collect(),
}
}
#[derive(Debug, Default)]
codex-rs/core-plugins/src/loader_tests.rs
-76
View File
@@ -218,82 +218,6 @@ enabled = true
assert!(hooks_only_valid.apps.is_empty());
}
#[test]
fn plugin_mcp_file_supports_mcp_servers_object_format() {
let parsed = serde_json::from_str::<PluginMcpFile>(
r#"{
"mcpServers": {
"sample": {
"command": "sample-mcp"
}
}
}"#,
)
.expect("parse wrapped plugin mcp config")
.into_mcp_servers();
assert_eq!(
parsed,
HashMap::from([(
"sample".to_string(),
serde_json::json!({
"command": "sample-mcp"
}),
)])
);
}
#[test]
fn plugin_mcp_file_supports_mcp_servers_object_format_with_metadata() {
let parsed = serde_json::from_str::<PluginMcpFile>(
r#"{
"$schema": "https://example.com/plugin-mcp.schema.json",
"mcpServers": {
"sample": {
"command": "sample-mcp"
}
}
}"#,
)
.expect("parse plugin mcp config with metadata")
.into_mcp_servers();
assert_eq!(
parsed,
HashMap::from([(
"sample".to_string(),
serde_json::json!({
"command": "sample-mcp"
}),
)])
);
}
#[test]
fn plugin_mcp_file_supports_top_level_server_map_format() {
let parsed = serde_json::from_str::<PluginMcpFile>(
r#"{
"linear": {
"type": "http",
"url": "https://mcp.linear.app/mcp"
}
}"#,
)
.expect("parse flat plugin mcp config")
.into_mcp_servers();
assert_eq!(
parsed,
HashMap::from([(
"linear".to_string(),
serde_json::json!({
"type": "http",
"url": "https://mcp.linear.app/mcp"
}),
)])
);
}
#[test]
fn curated_plugin_cache_version_shortens_full_git_sha() {
assert_eq!(