mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
17b9f4843e
## 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.
233 lines
7.9 KiB
Rust
233 lines
7.9 KiB
Rust
use codex_config::McpServerConfig;
|
|
use codex_config::McpServerEnvVar;
|
|
use codex_config::McpServerTransportConfig;
|
|
use serde::Deserialize;
|
|
use serde_json::Map as JsonMap;
|
|
use serde_json::Value as JsonValue;
|
|
use std::collections::BTreeMap;
|
|
use std::path::Component;
|
|
use std::path::Path;
|
|
use std::path::PathBuf;
|
|
use tracing::warn;
|
|
|
|
/// Placement applied while normalizing MCP servers declared by a plugin.
|
|
#[derive(Clone, Copy, Debug)]
|
|
pub enum PluginMcpServerPlacement<'a> {
|
|
/// Preserve declared placement, resolving a relative working directory below the plugin root.
|
|
Declared,
|
|
/// Bind stdio servers to one environment and default their working directory to the plugin root.
|
|
Environment { environment_id: &'a str },
|
|
}
|
|
|
|
/// One plugin MCP server that could not be normalized into runtime configuration.
|
|
#[derive(Clone, Debug, PartialEq, Eq)]
|
|
pub struct PluginMcpServerParseError {
|
|
pub name: String,
|
|
pub message: String,
|
|
}
|
|
|
|
/// Valid servers and per-server errors parsed from one plugin MCP file.
|
|
#[derive(Debug, Default, PartialEq)]
|
|
pub struct PluginMcpConfigParseOutcome {
|
|
pub servers: BTreeMap<String, McpServerConfig>,
|
|
pub errors: Vec<PluginMcpServerParseError>,
|
|
}
|
|
|
|
#[derive(Debug, Default, Deserialize)]
|
|
#[serde(rename_all = "camelCase")]
|
|
struct PluginMcpServersFile {
|
|
mcp_servers: BTreeMap<String, JsonValue>,
|
|
}
|
|
|
|
#[derive(Debug, Deserialize)]
|
|
#[serde(untagged)]
|
|
enum PluginMcpFile {
|
|
McpServersObject(PluginMcpServersFile),
|
|
ServerMap(BTreeMap<String, JsonValue>),
|
|
}
|
|
|
|
impl PluginMcpFile {
|
|
fn into_mcp_servers(self) -> BTreeMap<String, JsonValue> {
|
|
match self {
|
|
Self::McpServersObject(file) => file.mcp_servers,
|
|
Self::ServerMap(mcp_servers) => mcp_servers,
|
|
}
|
|
}
|
|
}
|
|
|
|
/// Parses the two supported plugin MCP file shapes and normalizes each server.
|
|
///
|
|
/// Invalid individual servers are returned as errors without discarding valid
|
|
/// siblings. A malformed top-level document fails the whole parse.
|
|
pub fn parse_plugin_mcp_config(
|
|
plugin_root: &Path,
|
|
contents: &str,
|
|
placement: PluginMcpServerPlacement<'_>,
|
|
) -> Result<PluginMcpConfigParseOutcome, serde_json::Error> {
|
|
let parsed = serde_json::from_str::<PluginMcpFile>(contents)?;
|
|
let mut outcome = PluginMcpConfigParseOutcome::default();
|
|
|
|
for (name, config_value) in parsed.into_mcp_servers() {
|
|
match normalize_plugin_mcp_server(plugin_root, config_value, placement) {
|
|
Ok(config) => {
|
|
outcome.servers.insert(name, config);
|
|
}
|
|
Err(message) => outcome
|
|
.errors
|
|
.push(PluginMcpServerParseError { name, message }),
|
|
}
|
|
}
|
|
|
|
Ok(outcome)
|
|
}
|
|
|
|
fn normalize_plugin_mcp_server(
|
|
plugin_root: &Path,
|
|
value: JsonValue,
|
|
placement: PluginMcpServerPlacement<'_>,
|
|
) -> Result<McpServerConfig, String> {
|
|
let mut object = normalize_plugin_mcp_server_value(plugin_root, value, placement);
|
|
if let PluginMcpServerPlacement::Environment { environment_id } = placement {
|
|
object.insert(
|
|
"environment_id".to_string(),
|
|
JsonValue::String(environment_id.to_string()),
|
|
);
|
|
if object.contains_key("command") {
|
|
match object.remove("cwd") {
|
|
Some(JsonValue::String(cwd)) => object.insert(
|
|
"cwd".to_string(),
|
|
JsonValue::String(
|
|
executor_plugin_cwd(plugin_root, &cwd)?
|
|
.to_string_lossy()
|
|
.into_owned(),
|
|
),
|
|
),
|
|
Some(JsonValue::Null) | None => object.insert(
|
|
"cwd".to_string(),
|
|
JsonValue::String(plugin_root.to_string_lossy().into_owned()),
|
|
),
|
|
Some(value) => object.insert("cwd".to_string(), value),
|
|
};
|
|
}
|
|
}
|
|
|
|
let mut config = serde_json::from_value::<McpServerConfig>(JsonValue::Object(object))
|
|
.map_err(|err| err.to_string())?;
|
|
if matches!(placement, PluginMcpServerPlacement::Environment { .. }) {
|
|
bind_environment_env_vars(&mut config)?;
|
|
}
|
|
Ok(config)
|
|
}
|
|
|
|
fn executor_plugin_cwd(plugin_root: &Path, configured_cwd: &str) -> Result<PathBuf, String> {
|
|
let cwd = Path::new(configured_cwd);
|
|
if cwd.is_absolute() {
|
|
return Ok(cwd.to_path_buf());
|
|
}
|
|
if cwd.components().any(|component| {
|
|
matches!(
|
|
component,
|
|
Component::ParentDir | Component::RootDir | Component::Prefix(_)
|
|
)
|
|
}) {
|
|
return Err(format!(
|
|
"relative cwd `{configured_cwd}` must remain within plugin root `{}`",
|
|
plugin_root.display()
|
|
));
|
|
}
|
|
Ok(plugin_root.join(cwd))
|
|
}
|
|
|
|
fn bind_environment_env_vars(config: &mut McpServerConfig) -> Result<(), String> {
|
|
let is_local_environment = config.is_local_environment();
|
|
let McpServerTransportConfig::Stdio { env_vars, .. } = &mut config.transport else {
|
|
return Ok(());
|
|
};
|
|
for env_var in env_vars {
|
|
match env_var {
|
|
McpServerEnvVar::Name(name) if !is_local_environment => {
|
|
*env_var = McpServerEnvVar::Config {
|
|
name: std::mem::take(name),
|
|
source: Some("remote".to_string()),
|
|
};
|
|
}
|
|
McpServerEnvVar::Name(_) => {}
|
|
McpServerEnvVar::Config { name, source } => {
|
|
match (is_local_environment, source.as_deref()) {
|
|
(true, None | Some("local")) | (false, Some("remote")) => {}
|
|
(true, Some("remote")) => {
|
|
return Err(format!(
|
|
"env_vars entry `{name}` cannot use source `remote` in a local environment"
|
|
));
|
|
}
|
|
(false, None) => *source = Some("remote".to_string()),
|
|
(false, Some("local")) => {
|
|
return Err(format!(
|
|
"env_vars entry `{name}` cannot use source `local` in an executor-owned plugin"
|
|
));
|
|
}
|
|
(_, Some(source)) => unreachable!("validated env_vars source `{source}`"),
|
|
}
|
|
}
|
|
}
|
|
}
|
|
Ok(())
|
|
}
|
|
|
|
fn normalize_plugin_mcp_server_value(
|
|
plugin_root: &Path,
|
|
value: JsonValue,
|
|
placement: PluginMcpServerPlacement<'_>,
|
|
) -> 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 matches!(placement, PluginMcpServerPlacement::Declared)
|
|
&& 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()),
|
|
);
|
|
}
|
|
|
|
object
|
|
}
|
|
|
|
#[cfg(test)]
|
|
#[path = "plugin_config_tests.rs"]
|
|
mod tests;
|