mirror of
https://github.com/openai/codex.git
synced 2026-10-30 19:18:49 +00:00
Scope agent MCP metadata to a host-selected plugin registration (#58378)
## What changed Add `AgentMcpMetadataRecipient` as a thread extension that lets the host select which MCP plugin registration receives `x-codex-root-turn-id` and `x-codex-cyber-access-program` in tool-call metadata. Require an exact match on the plugin ID, installed root, server name, and transport configuration in the `local` MCP environment. Omit both keys without host opt-in or when the registration does not match, including same-name user overrides. Send JSON null for unset values on matching registrations. ## Testing Add unit tests for registration matching and integration coverage for host opt-in, same-name user overrides, and an unset access program. GitOrigin-RevId: eaa44e17b29aa0bdec6f6ed9faa86b8fc931faad
This commit is contained in:
@@ -0,0 +1,41 @@
|
||||
//! Bind private agent attribution to one host-selected MCP plugin registration.
|
||||
|
||||
use codex_config::McpServerTransportConfig;
|
||||
use codex_mcp::McpServerSource;
|
||||
use codex_mcp::ResolvedMcpCatalog;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
|
||||
/// Install in thread extension data to send `x-codex-root-turn-id` and
|
||||
/// `x-codex-cyber-access-program` to every tool on this plugin registration.
|
||||
/// Only a registration from the host plugin manager in the `local` MCP environment can match.
|
||||
/// The full plugin config name (e.g. `agents@marketplace`), installed plugin root,
|
||||
/// server name, and loaded transport configuration must all match the effective registration.
|
||||
/// Core omits both keys without this attachment or an exact match. Same-name config
|
||||
/// overrides, thread-selected plugins, and extension registrations do not match.
|
||||
/// A matching registration receives JSON null for an unset value.
|
||||
/// Transport configuration may contain credentials and must not be logged.
|
||||
#[derive(Clone)]
|
||||
pub struct AgentMcpMetadataRecipient {
|
||||
pub plugin_id: String,
|
||||
pub plugin_root: PathUri,
|
||||
pub server_name: String,
|
||||
pub transport: McpServerTransportConfig,
|
||||
}
|
||||
|
||||
impl AgentMcpMetadataRecipient {
|
||||
pub(crate) fn matches(&self, server_name: &str, catalog: &ResolvedMcpCatalog) -> bool {
|
||||
let Some(registration) = catalog.server(server_name) else {
|
||||
return false;
|
||||
};
|
||||
server_name == self.server_name
|
||||
&& registration.config().is_local_environment()
|
||||
&& registration.config().transport == self.transport
|
||||
&& matches!(registration.source(), McpServerSource::Plugin(plugin)
|
||||
if plugin.plugin_id() == self.plugin_id
|
||||
&& plugin.host_root() == Some(&self.plugin_root))
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "mcp_metadata_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,60 @@
|
||||
#![cfg(test)]
|
||||
|
||||
use super::AgentMcpMetadataRecipient;
|
||||
use codex_config::McpServerConfig;
|
||||
use codex_mcp::McpPluginAttribution;
|
||||
use codex_mcp::McpServerRegistration;
|
||||
use codex_mcp::ResolvedMcpCatalog;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
|
||||
#[test_case::test_case("exact", true)]
|
||||
#[test_case::test_case("plugin_id", false)]
|
||||
#[test_case::test_case("plugin_root", false)]
|
||||
#[test_case::test_case("server_name", false)]
|
||||
#[test_case::test_case("endpoint", false)]
|
||||
#[test_case::test_case("environment", false)]
|
||||
#[test_case::test_case("config_source", false)]
|
||||
fn agent_context_requires_the_exact_captured_recipient(mismatch: &str, expected: bool) {
|
||||
let mut config: McpServerConfig = serde_json::from_value(json!({
|
||||
"url": "https://coordinator.example/mcp"
|
||||
}))
|
||||
.expect("server config");
|
||||
let recipient = AgentMcpMetadataRecipient {
|
||||
plugin_id: "agents@local".into(),
|
||||
plugin_root: PathUri::parse("file:///plugins/agents").unwrap(),
|
||||
server_name: "agents".into(),
|
||||
transport: config.transport.clone(),
|
||||
};
|
||||
let mut plugin_id = recipient.plugin_id.clone();
|
||||
let mut root = recipient.plugin_root.clone();
|
||||
let mut name = recipient.server_name.clone();
|
||||
match mismatch {
|
||||
"plugin_id" => plugin_id = "unrelated@local".into(),
|
||||
"plugin_root" => root = PathUri::parse("file:///plugins/replaced").unwrap(),
|
||||
"server_name" => name = "another-server".into(),
|
||||
"endpoint" => {
|
||||
config = serde_json::from_value(json!({
|
||||
"url": "https://unrelated.example/mcp"
|
||||
}))
|
||||
.expect("replacement server config")
|
||||
}
|
||||
"environment" => config.environment_id = "remote".into(),
|
||||
"exact" | "config_source" => {}
|
||||
_ => panic!("unknown mismatch"),
|
||||
}
|
||||
let registration = if mismatch == "config_source" {
|
||||
McpServerRegistration::from_config(name.clone(), config)
|
||||
} else {
|
||||
McpServerRegistration::from_plugin(
|
||||
name.clone(),
|
||||
McpPluginAttribution::new(plugin_id, "Agents".into()).with_host_root(root),
|
||||
/*plugin_order*/ 0,
|
||||
config,
|
||||
)
|
||||
};
|
||||
let mut builder = ResolvedMcpCatalog::builder();
|
||||
builder.register(registration);
|
||||
assert_eq!(recipient.matches(&name, &builder.build()), expected);
|
||||
}
|
||||
@@ -1,4 +1,5 @@
|
||||
mod invocation;
|
||||
mod mcp_metadata;
|
||||
mod spawn;
|
||||
mod types;
|
||||
|
||||
@@ -8,6 +9,7 @@ pub use invocation::ToolInvocationContext;
|
||||
pub(crate) use invocation::create;
|
||||
pub(crate) use invocation::prepare;
|
||||
pub(crate) use invocation::tool_invocation;
|
||||
pub use mcp_metadata::AgentMcpMetadataRecipient;
|
||||
pub(crate) use spawn::start_prepared;
|
||||
pub use types::HostThreadSpawnReservation;
|
||||
pub(crate) use types::PreparedChildInheritance;
|
||||
|
||||
@@ -153,6 +153,7 @@ mod event_mapping;
|
||||
pub use codex_prompts as review_prompts;
|
||||
mod thread_manager;
|
||||
mod thread_spawn;
|
||||
pub use agent::host::AgentMcpMetadataRecipient;
|
||||
pub use agent::host::HostThreadSpawnReservation;
|
||||
pub use agent::host::HostToolInvocationLookup;
|
||||
pub use agent::host::PreparedThreadSpawn;
|
||||
|
||||
@@ -542,13 +542,32 @@ async fn handle_approved_mcp_tool_call(
|
||||
if let Some(rewritten_arguments) = rewritten_arguments.as_ref() {
|
||||
tool_input = rewritten_arguments.clone();
|
||||
}
|
||||
let request_meta = build_mcp_tool_call_request_meta(
|
||||
let mut request_meta = build_mcp_tool_call_request_meta(
|
||||
step_context,
|
||||
&server,
|
||||
call_id,
|
||||
Some(&metadata),
|
||||
prepared_call.is_host_owned_apps(),
|
||||
);
|
||||
if let Some(recipient) = sess
|
||||
.services
|
||||
.thread_extension_data
|
||||
.get::<crate::AgentMcpMetadataRecipient>()
|
||||
&& recipient.matches(
|
||||
prepared_call.server_name(),
|
||||
&prepared_call.config().mcp_server_catalog,
|
||||
)
|
||||
&& let Some(JsonValue::Object(meta)) = &mut request_meta
|
||||
{
|
||||
meta.insert(
|
||||
"x-codex-root-turn-id".into(),
|
||||
serde_json::json!(turn_context.turn_metadata_state.root_turn_id()),
|
||||
);
|
||||
meta.insert(
|
||||
"x-codex-cyber-access-program".into(),
|
||||
serde_json::json!(turn_context.cyber_access_program),
|
||||
);
|
||||
}
|
||||
let request_meta = with_mcp_tool_call_ids_meta(
|
||||
request_meta,
|
||||
&sess.thread_id.to_string(),
|
||||
|
||||
@@ -25,9 +25,11 @@ use codex_config::types::McpServerConfig;
|
||||
use codex_config::types::McpServerEnvVar;
|
||||
use codex_config::types::McpServerTransportConfig;
|
||||
use codex_config::types::OAuthCredentialsStoreMode;
|
||||
use codex_core::AgentMcpMetadataRecipient;
|
||||
use codex_core::EnvironmentConfig;
|
||||
use codex_core::EnvironmentMcpPolicy;
|
||||
use codex_core::TurnInputRequest;
|
||||
use codex_core::TurnStartOptions;
|
||||
use codex_core::X_CODEX_ROUTING_HINT_HEADER;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::windows_sandbox::WindowsSandboxLevelExt;
|
||||
@@ -92,6 +94,7 @@ use codex_protocol::protocol::TurnSettingsUpdate;
|
||||
use codex_protocol::protocol::TurnSettingsUpdateOutcome;
|
||||
use codex_protocol::request_user_input::RequestUserInputAnswer;
|
||||
use codex_protocol::request_user_input::RequestUserInputResponse;
|
||||
use codex_protocol::turn_input::CyberAccessProgram;
|
||||
use codex_protocol::turn_input::TurnInput;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
@@ -413,7 +416,8 @@ async fn call_cwd_tool(
|
||||
server_name: &str,
|
||||
call_id: &str,
|
||||
) -> anyhow::Result<Value> {
|
||||
call_structured_tool(server, fixture, server_name, "cwd", call_id).await
|
||||
let start = TurnStartOptions::default();
|
||||
call_structured_tool(server, fixture, server_name, "cwd", call_id, start).await
|
||||
}
|
||||
|
||||
async fn call_structured_tool(
|
||||
@@ -422,6 +426,7 @@ async fn call_structured_tool(
|
||||
server_name: &str,
|
||||
tool_name: &str,
|
||||
call_id: &str,
|
||||
start: TurnStartOptions,
|
||||
) -> anyhow::Result<Value> {
|
||||
let namespace = format!("mcp__{server_name}");
|
||||
mount_sse_once(
|
||||
@@ -442,10 +447,8 @@ async fn call_structured_tool(
|
||||
)
|
||||
.await;
|
||||
|
||||
fixture
|
||||
.codex
|
||||
.start_or_steer_turn(read_only_user_turn(fixture, "call the requested rmcp tool"))
|
||||
.await?;
|
||||
let input = read_only_user_turn(fixture, "call the requested rmcp tool").on_start(start);
|
||||
fixture.codex.start_or_steer_turn(input).await?;
|
||||
|
||||
wait_for_event(&fixture.codex, |ev| {
|
||||
matches!(ev, EventMsg::McpToolCallBegin(_))
|
||||
@@ -471,6 +474,85 @@ async fn call_structured_tool(
|
||||
Ok(structured_content)
|
||||
}
|
||||
|
||||
enum AgentMetadataCase {
|
||||
ApprovedPlugin,
|
||||
NoHostOptIn,
|
||||
UserOverride,
|
||||
}
|
||||
|
||||
#[test_case(AgentMetadataCase::ApprovedPlugin, Some(CyberAccessProgram::Standard); "approved plugin")]
|
||||
#[test_case(AgentMetadataCase::ApprovedPlugin, None; "approved plugin without an explicit program")]
|
||||
#[test_case(AgentMetadataCase::UserOverride, Some(CyberAccessProgram::Standard); "same-name user override")]
|
||||
#[test_case(AgentMetadataCase::NoHostOptIn, Some(CyberAccessProgram::Standard); "matching plugin without host opt-in")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn agent_metadata_is_only_sent_to_the_selected_plugin(
|
||||
case: AgentMetadataCase,
|
||||
cyber_access_program: Option<CyberAccessProgram>,
|
||||
) -> anyhow::Result<()> {
|
||||
skip_if_wine_exec!(
|
||||
Ok(()),
|
||||
"requires a Windows test_stdio_server in the Wine-exec environment"
|
||||
);
|
||||
let server = start_mock_server().await;
|
||||
let home = Arc::new(tempdir()?);
|
||||
let plugin_root = super::plugins::write_sample_plugin_manifest_and_config(&home);
|
||||
let command = stdio_server_bin()?;
|
||||
fs::write(
|
||||
plugin_root.join(".mcp.json"),
|
||||
serde_json::to_vec(&json!({ "mcpServers": { "sample": { "command": command } } }))?,
|
||||
)?;
|
||||
let transport = stdio_transport(command, /*env*/ None, Vec::new());
|
||||
let recipient = AgentMcpMetadataRecipient {
|
||||
plugin_id: "sample@test".into(),
|
||||
plugin_root: PathUri::from_abs_path(&AbsolutePathBuf::try_from(plugin_root)?),
|
||||
server_name: "sample".into(),
|
||||
transport: transport.clone(),
|
||||
};
|
||||
let user_override = matches!(case, AgentMetadataCase::UserOverride);
|
||||
let fixture = test_codex()
|
||||
.with_home(home)
|
||||
.with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing())
|
||||
.with_config(move |config| {
|
||||
if user_override {
|
||||
insert_mcp_server(config, "sample", transport, TestMcpServerOptions::default());
|
||||
}
|
||||
})
|
||||
.build(&server)
|
||||
.await?;
|
||||
if !matches!(case, AgentMetadataCase::NoHostOptIn) {
|
||||
fixture.codex.thread_extension_data().insert(recipient);
|
||||
}
|
||||
wait_for_mcp_server(&fixture.codex, "sample").await?;
|
||||
|
||||
let call_id = "call-agent-context";
|
||||
let meta = call_structured_tool(
|
||||
&server,
|
||||
&fixture,
|
||||
"sample",
|
||||
"sandbox_meta",
|
||||
call_id,
|
||||
TurnStartOptions {
|
||||
root_turn_id: Some("original-root".into()),
|
||||
turn_trigger: Some("composer".into()),
|
||||
cyber_access_program,
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
assert_eq!(meta["callId"], json!(call_id));
|
||||
assert_eq!(meta["x-codex-turn-metadata"]["turn_trigger"], "composer");
|
||||
let expect_private_metadata = matches!(case, AgentMetadataCase::ApprovedPlugin);
|
||||
assert_eq!(
|
||||
meta.get("x-codex-root-turn-id").cloned(),
|
||||
expect_private_metadata.then(|| json!("original-root"))
|
||||
);
|
||||
assert_eq!(
|
||||
meta.get("x-codex-cyber-access-program").cloned(),
|
||||
expect_private_metadata.then(|| json!(cyber_access_program))
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
|
||||
async fn openai_form_capability_is_advertised_to_mcp_servers() -> anyhow::Result<()> {
|
||||
assert_openai_form_capability_advertisement(/*expected*/ true).await
|
||||
@@ -510,6 +592,7 @@ async fn assert_openai_form_capability_advertisement(expected: bool) -> anyhow::
|
||||
server_name,
|
||||
"client_capabilities",
|
||||
"call-client-capabilities",
|
||||
Default::default(),
|
||||
)
|
||||
.await?;
|
||||
assert_eq!(
|
||||
|
||||
Reference in New Issue
Block a user