mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
## Why MCP servers currently come from user config, local plugins, compatibility Apps synthesis, and host extensions. Those sources were composed by mutating a shared map, leaving registration identity, precedence, removal, and provenance implicit in assembly order. Before adding executor-owned MCPs, Codex needs one durable resolution boundary above `McpConnectionManager`. This PR introduces that boundary while preserving current server configuration, policy, and runtime behavior. Executor-scoped registrations and explicit policy layers remain follow-ups. ## What changed - Add typed `McpServerRegistration` inputs and an immutable `ResolvedMcpCatalog` in `codex-mcp`. - Retain each registration's complete `McpServerConfig`, including its environment binding, while recording its source and provenance. - Preserve the existing structural precedence between plugin, config, compatibility, and ordered extension sources. - Resolve equal-precedence actions by contribution order; provenance IDs are used only for diagnostics and cannot affect the winner. - Preserve extension removals and the existing name-scoped `enabled = false` veto. - Report same-tier conflicts with every contender and the final catalog outcome, including whether the winning action registers or removes the server. - Require MCP contributors to provide a stable diagnostic identity. - Derive materialized server maps and plugin ownership from the resolved catalog. `McpConnectionManager`, transport startup, tool calls, and resource routing continue to consume the same effective `McpServerConfig` values. ## Scope This PR does not add new MCP capabilities or change user-visible behavior. It does not add executor plugin discovery, thread-scoped registrations, dynamic refresh generations, or new user/managed policy semantics. ## Verification - Added focused catalog coverage for source precedence, complete configuration preservation, disabled vetoes, plugin ownership, contribution-order tie breaking, removal outcomes, and conflict diagnostics. - Extended hosted Apps coverage for ordered extension removal and Apps-disabled hosts with and without the hosted extension installed. - `cargo check -p codex-mcp --tests -p codex-extension-api -p codex-core`
258 lines
7.7 KiB
Rust
258 lines
7.7 KiB
Rust
use std::collections::HashMap;
|
|
use std::time::Duration;
|
|
|
|
use codex_config::AppToolApproval;
|
|
use codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID;
|
|
use codex_config::McpServerConfig;
|
|
use codex_config::McpServerToolConfig;
|
|
use codex_config::McpServerTransportConfig;
|
|
use pretty_assertions::assert_eq;
|
|
|
|
use super::McpServerConflict;
|
|
use super::McpServerConflictAction;
|
|
use super::McpServerRegistration;
|
|
use super::McpServerSource;
|
|
use super::ResolvedMcpCatalog;
|
|
|
|
fn server(url: &str) -> McpServerConfig {
|
|
McpServerConfig {
|
|
transport: McpServerTransportConfig::StreamableHttp {
|
|
url: url.to_string(),
|
|
bearer_token_env_var: None,
|
|
http_headers: None,
|
|
env_http_headers: None,
|
|
},
|
|
environment_id: DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(),
|
|
enabled: true,
|
|
required: true,
|
|
supports_parallel_tool_calls: true,
|
|
disabled_reason: None,
|
|
startup_timeout_sec: Some(Duration::from_secs(7)),
|
|
tool_timeout_sec: Some(Duration::from_secs(11)),
|
|
default_tools_approval_mode: Some(AppToolApproval::Prompt),
|
|
enabled_tools: Some(vec!["read".to_string()]),
|
|
disabled_tools: Some(vec!["write".to_string()]),
|
|
scopes: None,
|
|
oauth: None,
|
|
oauth_resource: None,
|
|
tools: HashMap::from([(
|
|
"read".to_string(),
|
|
McpServerToolConfig {
|
|
approval_mode: Some(AppToolApproval::Approve),
|
|
},
|
|
)]),
|
|
}
|
|
}
|
|
|
|
fn plugin_source(plugin_id: &str) -> McpServerSource {
|
|
McpServerSource::Plugin {
|
|
plugin_id: plugin_id.to_string(),
|
|
}
|
|
}
|
|
|
|
fn compatibility_source(id: &str) -> McpServerSource {
|
|
McpServerSource::Compatibility { id: id.to_string() }
|
|
}
|
|
|
|
fn extension_source(id: &str) -> McpServerSource {
|
|
McpServerSource::Extension { id: id.to_string() }
|
|
}
|
|
|
|
fn register(source: McpServerSource) -> McpServerConflictAction {
|
|
McpServerConflictAction::Register(source)
|
|
}
|
|
|
|
fn remove(source: McpServerSource) -> McpServerConflictAction {
|
|
McpServerConflictAction::Remove(source)
|
|
}
|
|
|
|
#[test]
|
|
fn source_precedence_preserves_the_winning_registration() {
|
|
let extension = server("https://extension.example/mcp");
|
|
let mut plugin = server("https://plugin.example/mcp");
|
|
plugin.enabled = false;
|
|
let mut builder = ResolvedMcpCatalog::builder();
|
|
builder.register(McpServerRegistration::from_extension(
|
|
"docs".to_string(),
|
|
"hosted",
|
|
/*contribution_order*/ 0,
|
|
extension.clone(),
|
|
));
|
|
builder.register(McpServerRegistration::from_plugin(
|
|
"docs".to_string(),
|
|
"plugin@test".to_string(),
|
|
/*plugin_order*/ 0,
|
|
plugin,
|
|
));
|
|
builder.register(McpServerRegistration::from_plugin(
|
|
"docs".to_string(),
|
|
"other-plugin@test".to_string(),
|
|
/*plugin_order*/ 1,
|
|
server("https://other-plugin.example/mcp"),
|
|
));
|
|
builder.register(McpServerRegistration::from_compatibility(
|
|
"docs".to_string(),
|
|
"legacy",
|
|
server("https://compatibility.example/mcp"),
|
|
));
|
|
builder.register(McpServerRegistration::from_config(
|
|
"docs".to_string(),
|
|
server("https://config.example/mcp"),
|
|
));
|
|
|
|
let catalog = builder.build();
|
|
let resolved = catalog.server("docs").expect("resolved server");
|
|
|
|
assert_eq!(
|
|
resolved.source(),
|
|
&McpServerSource::Extension {
|
|
id: "hosted".to_string(),
|
|
}
|
|
);
|
|
assert_eq!(resolved.config(), &extension);
|
|
assert!(catalog.plugin_ids_by_server_name().is_empty());
|
|
assert_eq!(
|
|
catalog.conflicts(),
|
|
&[McpServerConflict {
|
|
name: "docs".to_string(),
|
|
outcome: register(extension_source("hosted")),
|
|
contenders: vec![
|
|
register(plugin_source("other-plugin@test")),
|
|
register(plugin_source("plugin@test")),
|
|
],
|
|
}]
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn disabled_veto_only_disables_the_winning_registration() {
|
|
let extension = server("https://extension.example/mcp");
|
|
let mut expected = extension.clone();
|
|
expected.enabled = false;
|
|
let mut builder = ResolvedMcpCatalog::builder();
|
|
builder.register(McpServerRegistration::from_extension(
|
|
"docs".to_string(),
|
|
"hosted",
|
|
/*contribution_order*/ 0,
|
|
extension,
|
|
));
|
|
builder.disable("docs".to_string());
|
|
|
|
let actual = builder
|
|
.build()
|
|
.server("docs")
|
|
.expect("resolved server")
|
|
.config()
|
|
.clone();
|
|
|
|
assert_eq!(actual, expected);
|
|
}
|
|
|
|
#[test]
|
|
fn disabled_winner_remains_a_veto_when_the_catalog_is_extended() {
|
|
let mut disabled = server("https://config.example/mcp");
|
|
disabled.enabled = false;
|
|
let mut expected = server("https://extension.example/mcp");
|
|
expected.enabled = false;
|
|
let mut builder = ResolvedMcpCatalog::builder();
|
|
builder.register(McpServerRegistration::from_config(
|
|
"docs".to_string(),
|
|
disabled,
|
|
));
|
|
let mut builder = builder.build().to_builder();
|
|
builder.register(McpServerRegistration::from_extension(
|
|
"docs".to_string(),
|
|
"hosted",
|
|
/*contribution_order*/ 0,
|
|
server("https://extension.example/mcp"),
|
|
));
|
|
|
|
let resolved = builder.build();
|
|
|
|
assert_eq!(
|
|
resolved.server("docs"),
|
|
Some(&super::ResolvedMcpServer {
|
|
source: extension_source("hosted"),
|
|
config: expected,
|
|
})
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn earlier_plugin_wins_with_an_explicit_conflict() {
|
|
let mut builder = ResolvedMcpCatalog::builder();
|
|
builder.register(McpServerRegistration::from_plugin(
|
|
"docs".to_string(),
|
|
"alpha@test".to_string(),
|
|
/*plugin_order*/ 0,
|
|
server("https://alpha.example/mcp"),
|
|
));
|
|
builder.register(McpServerRegistration::from_plugin(
|
|
"docs".to_string(),
|
|
"beta@test".to_string(),
|
|
/*plugin_order*/ 1,
|
|
server("https://beta.example/mcp"),
|
|
));
|
|
|
|
let catalog = builder.build();
|
|
|
|
assert_eq!(
|
|
catalog.plugin_ids_by_server_name(),
|
|
HashMap::from([("docs".to_string(), "alpha@test".to_string())])
|
|
);
|
|
assert_eq!(
|
|
catalog.conflicts(),
|
|
&[McpServerConflict {
|
|
name: "docs".to_string(),
|
|
outcome: register(plugin_source("alpha@test")),
|
|
contenders: vec![
|
|
register(plugin_source("beta@test")),
|
|
register(plugin_source("alpha@test")),
|
|
],
|
|
}]
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn equal_precedence_uses_insertion_order_not_source_identity() {
|
|
let mut builder = ResolvedMcpCatalog::builder();
|
|
builder.register(McpServerRegistration::from_compatibility(
|
|
"docs".to_string(),
|
|
"z-first",
|
|
server("https://first.example/mcp"),
|
|
));
|
|
builder.register(McpServerRegistration::from_compatibility(
|
|
"docs".to_string(),
|
|
"a-second",
|
|
server("https://second.example/mcp"),
|
|
));
|
|
|
|
let catalog = builder.build();
|
|
|
|
assert_eq!(
|
|
catalog.server("docs"),
|
|
Some(&super::ResolvedMcpServer {
|
|
source: compatibility_source("a-second"),
|
|
config: server("https://second.example/mcp"),
|
|
})
|
|
);
|
|
let mut builder = catalog.to_builder();
|
|
builder.remove_compatibility("docs".to_string(), "remove-last");
|
|
|
|
let catalog = builder.build();
|
|
|
|
assert_eq!(catalog.server("docs"), None);
|
|
assert_eq!(
|
|
catalog.conflicts(),
|
|
&[McpServerConflict {
|
|
name: "docs".to_string(),
|
|
outcome: remove(compatibility_source("remove-last")),
|
|
contenders: vec![
|
|
register(compatibility_source("z-first")),
|
|
register(compatibility_source("a-second")),
|
|
remove(compatibility_source("remove-last")),
|
|
],
|
|
}]
|
|
);
|
|
}
|