From 6b0ed01fed1987b79026789b70183b7330bf7563 Mon Sep 17 00:00:00 2001 From: Dylan Hurd Date: Fri, 17 Apr 2026 13:24:50 -0700 Subject: [PATCH] Add spawned agent MCP allowlists Support configuring MCP server allowlists for spawned agents through agents.spawn.mcp_servers and per-call spawn_agent overrides. Apply the allowlist when resolving effective MCP servers, including the built-in apps server, and reject unknown server names before spawning. Co-authored-by: Codex --- codex-rs/codex-mcp/src/mcp/mod.rs | 26 +- codex-rs/codex-mcp/src/mcp/mod_tests.rs | 101 +++-- codex-rs/config/src/config_toml.rs | 11 + codex-rs/core/config.schema.json | 21 + codex-rs/core/src/config/config_tests.rs | 66 +++ codex-rs/core/src/config/mod.rs | 62 +++ .../src/tools/handlers/multi_agents/spawn.rs | 8 + .../src/tools/handlers/multi_agents_common.rs | 90 +++- .../src/tools/handlers/multi_agents_tests.rs | 394 +++++++++++++++++- .../tools/handlers/multi_agents_v2/spawn.rs | 8 + codex-rs/tools/src/agent_tool.rs | 20 + codex-rs/tools/src/agent_tool_tests.rs | 2 + .../tools/src/tool_registry_plan_tests.rs | 2 + docs/config.md | 16 + 14 files changed, 775 insertions(+), 52 deletions(-) diff --git a/codex-rs/codex-mcp/src/mcp/mod.rs b/codex-rs/codex-mcp/src/mcp/mod.rs index 1dc9db0789..c326099749 100644 --- a/codex-rs/codex-mcp/src/mcp/mod.rs +++ b/codex-rs/codex-mcp/src/mcp/mod.rs @@ -14,6 +14,7 @@ pub use skill_dependencies::canonical_mcp_server_key; pub use skill_dependencies::collect_missing_mcp_dependencies; use std::collections::HashMap; +use std::collections::HashSet; use std::env; use std::path::PathBuf; use std::time::Duration; @@ -131,6 +132,8 @@ pub struct McpConfig { pub apps_enabled: bool, /// User-configured and plugin-provided MCP servers keyed by server name. pub configured_mcp_servers: HashMap, + /// Optional allowlist of MCP server names for the current session. + pub mcp_server_allowlist: Option>, /// Plugin metadata used to attribute MCP tools/connectors to plugin display names. pub plugin_capability_summaries: Vec, } @@ -298,19 +301,34 @@ pub fn with_codex_apps_mcp( } else { servers.remove(CODEX_APPS_MCP_SERVER_NAME); } - servers + filter_mcp_servers_by_allowlist(servers, config.mcp_server_allowlist.as_deref()) } pub fn configured_mcp_servers(config: &McpConfig) -> HashMap { - config.configured_mcp_servers.clone() + filter_mcp_servers_by_allowlist( + config.configured_mcp_servers.clone(), + config.mcp_server_allowlist.as_deref(), + ) } pub fn effective_mcp_servers( config: &McpConfig, auth: Option<&CodexAuth>, ) -> HashMap { - let servers = configured_mcp_servers(config); - with_codex_apps_mcp(servers, auth, config) + with_codex_apps_mcp(config.configured_mcp_servers.clone(), auth, config) +} + +fn filter_mcp_servers_by_allowlist( + mut servers: HashMap, + allowlist: Option<&[String]>, +) -> HashMap { + let Some(allowlist) = allowlist else { + return servers; + }; + + let allowlist = allowlist.iter().map(String::as_str).collect::>(); + servers.retain(|name, _| allowlist.contains(name.as_str())); + servers } pub fn tool_plugin_provenance(config: &McpConfig) -> ToolPluginProvenance { diff --git a/codex-rs/codex-mcp/src/mcp/mod_tests.rs b/codex-rs/codex-mcp/src/mcp/mod_tests.rs index 8db52a9d83..7d0cc23491 100644 --- a/codex-rs/codex-mcp/src/mcp/mod_tests.rs +++ b/codex-rs/codex-mcp/src/mcp/mod_tests.rs @@ -21,10 +21,35 @@ fn test_mcp_config(codex_home: PathBuf) -> McpConfig { use_legacy_landlock: false, apps_enabled: false, configured_mcp_servers: HashMap::new(), + mcp_server_allowlist: None, plugin_capability_summaries: Vec::new(), } } +fn http_mcp_server(url: &str) -> McpServerConfig { + McpServerConfig { + transport: McpServerTransportConfig::StreamableHttp { + url: url.to_string(), + bearer_token_env_var: None, + http_headers: None, + env_http_headers: None, + }, + experimental_environment: None, + enabled: true, + required: false, + supports_parallel_tool_calls: false, + disabled_reason: None, + startup_timeout_sec: None, + tool_timeout_sec: None, + default_tools_approval_mode: None, + enabled_tools: None, + disabled_tools: None, + scopes: None, + oauth_resource: None, + tools: HashMap::new(), + } +} + fn make_tool(name: &str) -> Tool { Tool { name: name.to_string(), @@ -186,51 +211,11 @@ async fn effective_mcp_servers_preserve_user_servers_and_add_codex_apps() { config.configured_mcp_servers.insert( "sample".to_string(), - McpServerConfig { - transport: McpServerTransportConfig::StreamableHttp { - url: "https://user.example/mcp".to_string(), - bearer_token_env_var: None, - http_headers: None, - env_http_headers: None, - }, - experimental_environment: None, - enabled: true, - required: false, - supports_parallel_tool_calls: false, - disabled_reason: None, - startup_timeout_sec: None, - tool_timeout_sec: None, - default_tools_approval_mode: None, - enabled_tools: None, - disabled_tools: None, - scopes: None, - oauth_resource: None, - tools: HashMap::new(), - }, + http_mcp_server("https://user.example/mcp"), ); config.configured_mcp_servers.insert( "docs".to_string(), - McpServerConfig { - transport: McpServerTransportConfig::StreamableHttp { - url: "https://docs.example/mcp".to_string(), - bearer_token_env_var: None, - http_headers: None, - env_http_headers: None, - }, - experimental_environment: None, - enabled: true, - required: false, - supports_parallel_tool_calls: false, - disabled_reason: None, - startup_timeout_sec: None, - tool_timeout_sec: None, - default_tools_approval_mode: None, - enabled_tools: None, - disabled_tools: None, - scopes: None, - oauth_resource: None, - tools: HashMap::new(), - }, + http_mcp_server("https://docs.example/mcp"), ); let effective = effective_mcp_servers(&config, Some(&auth)); @@ -262,3 +247,35 @@ async fn effective_mcp_servers_preserve_user_servers_and_add_codex_apps() { other => panic!("expected streamable http transport, got {other:?}"), } } + +#[tokio::test] +async fn effective_mcp_servers_respects_session_allowlist() { + let mut config = test_mcp_config(PathBuf::from("/tmp")); + config.apps_enabled = true; + config.configured_mcp_servers.insert( + "docs".to_string(), + http_mcp_server("https://docs.example/mcp"), + ); + config.configured_mcp_servers.insert( + "linear".to_string(), + http_mcp_server("https://linear.example/mcp"), + ); + config.mcp_server_allowlist = Some(vec![ + "docs".to_string(), + CODEX_APPS_MCP_SERVER_NAME.to_string(), + ]); + + let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); + let effective = effective_mcp_servers(&config, Some(&auth)); + + assert_eq!( + effective + .keys() + .cloned() + .collect::>(), + std::collections::BTreeSet::from([ + "docs".to_string(), + CODEX_APPS_MCP_SERVER_NAME.to_string(), + ]) + ); +} diff --git a/codex-rs/config/src/config_toml.rs b/codex-rs/config/src/config_toml.rs index 83fe6c88e7..1d47eee4fb 100644 --- a/codex-rs/config/src/config_toml.rs +++ b/codex-rs/config/src/config_toml.rs @@ -543,6 +543,9 @@ pub struct AgentsToml { #[schemars(range(min = 1))] pub job_max_runtime_seconds: Option, + /// Defaults applied to agents when they are spawned. + pub spawn: Option, + /// User-defined role declarations keyed by role name. /// /// Example: @@ -556,6 +559,14 @@ pub struct AgentsToml { pub roles: BTreeMap, } +#[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)] +#[schemars(deny_unknown_fields)] +pub struct AgentSpawnToml { + /// MCP server names to start in spawned agents. When unset, spawned agents inherit all + /// effective MCP servers. An empty list disables MCP servers for spawned agents. + pub mcp_servers: Option>, +} + #[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)] #[schemars(deny_unknown_fields)] pub struct AgentRoleToml { diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 2eb8cf7270..9a6b8ca61d 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -31,6 +31,19 @@ }, "type": "object" }, + "AgentSpawnToml": { + "additionalProperties": false, + "properties": { + "mcp_servers": { + "description": "MCP server names to start in spawned agents. When unset, spawned agents inherit all effective MCP servers. An empty list disables MCP servers for spawned agents.", + "items": { + "type": "string" + }, + "type": "array" + } + }, + "type": "object" + }, "AgentsToml": { "additionalProperties": { "$ref": "#/definitions/AgentRoleToml" @@ -53,6 +66,14 @@ "format": "uint", "minimum": 1.0, "type": "integer" + }, + "spawn": { + "allOf": [ + { + "$ref": "#/definitions/AgentSpawnToml" + } + ], + "description": "Defaults applied to agents when they are spawned." } }, "type": "object" diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index c40d49fb88..c3a11e7109 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -3547,6 +3547,7 @@ async fn load_config_rejects_missing_agent_role_config_file() -> std::io::Result max_threads: None, max_depth: None, job_max_runtime_seconds: None, + spawn: None, roles: BTreeMap::from([( "researcher".to_string(), AgentRoleToml { @@ -4413,6 +4414,7 @@ async fn load_config_normalizes_agent_role_nickname_candidates() -> std::io::Res max_threads: None, max_depth: None, job_max_runtime_seconds: None, + spawn: None, roles: BTreeMap::from([( "researcher".to_string(), AgentRoleToml { @@ -4455,6 +4457,7 @@ async fn load_config_rejects_empty_agent_role_nickname_candidates() -> std::io:: max_threads: None, max_depth: None, job_max_runtime_seconds: None, + spawn: None, roles: BTreeMap::from([( "researcher".to_string(), AgentRoleToml { @@ -4491,6 +4494,7 @@ async fn load_config_rejects_duplicate_agent_role_nickname_candidates() -> std:: max_threads: None, max_depth: None, job_max_runtime_seconds: None, + spawn: None, roles: BTreeMap::from([( "researcher".to_string(), AgentRoleToml { @@ -4527,6 +4531,7 @@ async fn load_config_rejects_unsafe_agent_role_nickname_candidates() -> std::io: max_threads: None, max_depth: None, job_max_runtime_seconds: None, + spawn: None, roles: BTreeMap::from([( "researcher".to_string(), AgentRoleToml { @@ -4767,6 +4772,7 @@ async fn test_precedence_fixture_with_o3_profile() -> std::io::Result<()> { cwd: fixture.cwd(), cli_auth_credentials_store_mode: Default::default(), mcp_servers: Constrained::allow_any(HashMap::new()), + mcp_server_allowlist: None, mcp_oauth_credentials_store_mode: resolve_mcp_oauth_credentials_store_mode( Default::default(), LOCAL_DEV_BUILD_VERSION, @@ -4779,6 +4785,7 @@ async fn test_precedence_fixture_with_o3_profile() -> std::io::Result<()> { tool_output_token_limit: None, agent_max_threads: DEFAULT_AGENT_MAX_THREADS, agent_max_depth: DEFAULT_AGENT_MAX_DEPTH, + agent_spawn: AgentSpawnConfig::default(), agent_roles: BTreeMap::new(), memories: MemoriesConfig::default(), agent_job_max_runtime_seconds: DEFAULT_AGENT_JOB_MAX_RUNTIME_SECONDS, @@ -4917,6 +4924,7 @@ async fn test_precedence_fixture_with_gpt3_profile() -> std::io::Result<()> { cwd: fixture.cwd(), cli_auth_credentials_store_mode: Default::default(), mcp_servers: Constrained::allow_any(HashMap::new()), + mcp_server_allowlist: None, mcp_oauth_credentials_store_mode: resolve_mcp_oauth_credentials_store_mode( Default::default(), LOCAL_DEV_BUILD_VERSION, @@ -4929,6 +4937,7 @@ async fn test_precedence_fixture_with_gpt3_profile() -> std::io::Result<()> { tool_output_token_limit: None, agent_max_threads: DEFAULT_AGENT_MAX_THREADS, agent_max_depth: DEFAULT_AGENT_MAX_DEPTH, + agent_spawn: AgentSpawnConfig::default(), agent_roles: BTreeMap::new(), memories: MemoriesConfig::default(), agent_job_max_runtime_seconds: DEFAULT_AGENT_JOB_MAX_RUNTIME_SECONDS, @@ -5065,6 +5074,7 @@ async fn test_precedence_fixture_with_zdr_profile() -> std::io::Result<()> { cwd: fixture.cwd(), cli_auth_credentials_store_mode: Default::default(), mcp_servers: Constrained::allow_any(HashMap::new()), + mcp_server_allowlist: None, mcp_oauth_credentials_store_mode: resolve_mcp_oauth_credentials_store_mode( Default::default(), LOCAL_DEV_BUILD_VERSION, @@ -5077,6 +5087,7 @@ async fn test_precedence_fixture_with_zdr_profile() -> std::io::Result<()> { tool_output_token_limit: None, agent_max_threads: DEFAULT_AGENT_MAX_THREADS, agent_max_depth: DEFAULT_AGENT_MAX_DEPTH, + agent_spawn: AgentSpawnConfig::default(), agent_roles: BTreeMap::new(), memories: MemoriesConfig::default(), agent_job_max_runtime_seconds: DEFAULT_AGENT_JOB_MAX_RUNTIME_SECONDS, @@ -5198,6 +5209,7 @@ async fn test_precedence_fixture_with_gpt5_profile() -> std::io::Result<()> { cwd: fixture.cwd(), cli_auth_credentials_store_mode: Default::default(), mcp_servers: Constrained::allow_any(HashMap::new()), + mcp_server_allowlist: None, mcp_oauth_credentials_store_mode: resolve_mcp_oauth_credentials_store_mode( Default::default(), LOCAL_DEV_BUILD_VERSION, @@ -5210,6 +5222,7 @@ async fn test_precedence_fixture_with_gpt5_profile() -> std::io::Result<()> { tool_output_token_limit: None, agent_max_threads: DEFAULT_AGENT_MAX_THREADS, agent_max_depth: DEFAULT_AGENT_MAX_DEPTH, + agent_spawn: AgentSpawnConfig::default(), agent_roles: BTreeMap::new(), memories: MemoriesConfig::default(), agent_job_max_runtime_seconds: DEFAULT_AGENT_JOB_MAX_RUNTIME_SECONDS, @@ -6509,6 +6522,59 @@ hide_spawn_agent_metadata = false Ok(()) } +#[tokio::test] +async fn agents_spawn_config_sets_child_defaults_only() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + std::fs::write( + codex_home.path().join(CONFIG_TOML_FILE), + r#"[agents.spawn] +mcp_servers = ["docs", " linear ", "docs"] +"#, + )?; + + let config = ConfigBuilder::without_managed_config_for_tests() + .codex_home(codex_home.path().to_path_buf()) + .fallback_cwd(Some(codex_home.path().to_path_buf())) + .build() + .await?; + + assert_eq!( + config.agent_spawn, + AgentSpawnConfig { + mcp_servers: Some(vec!["docs".to_string(), "linear".to_string()]), + } + ); + assert_eq!(config.mcp_server_allowlist, None); + + Ok(()) +} + +#[tokio::test] +async fn agents_spawn_config_rejects_blank_mcp_server_names() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + std::fs::write( + codex_home.path().join(CONFIG_TOML_FILE), + r#"[agents.spawn] +mcp_servers = ["docs", " "] +"#, + )?; + + let err = ConfigBuilder::without_managed_config_for_tests() + .codex_home(codex_home.path().to_path_buf()) + .fallback_cwd(Some(codex_home.path().to_path_buf())) + .build() + .await + .expect_err("blank MCP server names should be rejected"); + + assert!( + err.to_string() + .contains("agents.spawn.mcp_servers cannot contain blank MCP server names"), + "unexpected error: {err}" + ); + + Ok(()) +} + #[tokio::test] async fn feature_requirements_normalize_runtime_feature_mutations() -> std::io::Result<()> { let codex_home = TempDir::new()?; diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 6320bff2e3..3cb69f296f 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -21,6 +21,7 @@ use crate::unified_exec::MIN_EMPTY_YIELD_TIME_MS; use crate::windows_sandbox::WindowsSandboxLevelExt; use crate::windows_sandbox::resolve_windows_sandbox_mode; use crate::windows_sandbox::resolve_windows_sandbox_private_desktop; +use codex_config::config_toml::AgentSpawnToml; use codex_config::config_toml::ConfigToml; use codex_config::config_toml::ProjectConfig; use codex_config::config_toml::RealtimeAudioConfig; @@ -376,6 +377,9 @@ pub struct Config { /// Definition for MCP servers that Codex can reach out to for tool calls. pub mcp_servers: Constrained>, + /// Optional MCP server allowlist for the current session. + pub mcp_server_allowlist: Option>, + /// Preferred store for MCP OAuth credentials. /// keyring: Use an OS-specific keyring service. /// Credentials stored in the keyring will only be readable by Codex unless the user explicitly grants access via OS-level keyring access. @@ -417,6 +421,9 @@ pub struct Config { /// Maximum nesting depth allowed for spawned agent threads. pub agent_max_depth: i32, + /// Defaults applied when this session spawns child agents. + pub agent_spawn: AgentSpawnConfig, + /// User-defined role declarations keyed by role name. pub agent_roles: BTreeMap, @@ -604,6 +611,17 @@ pub struct MultiAgentV2Config { pub hide_spawn_agent_metadata: bool, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct AgentSpawnConfig { + pub mcp_servers: Option>, +} + +impl Default for AgentSpawnConfig { + fn default() -> Self { + Self { mcp_servers: None } + } +} + impl Default for MultiAgentV2Config { fn default() -> Self { Self { @@ -774,6 +792,7 @@ impl Config { use_legacy_landlock: self.features.use_legacy_landlock(), apps_enabled: self.features.enabled(Feature::Apps), configured_mcp_servers, + mcp_server_allowlist: self.mcp_server_allowlist.clone(), plugin_capability_summaries: loaded_plugins.capability_summaries().to_vec(), } } @@ -1407,6 +1426,42 @@ fn resolve_multi_agent_v2_config( } } +fn resolve_agent_spawn_config(spawn: Option<&AgentSpawnToml>) -> std::io::Result { + let Some(spawn) = spawn else { + return Ok(AgentSpawnConfig { mcp_servers: None }); + }; + + let mcp_servers = spawn + .mcp_servers + .as_ref() + .map(|servers| normalize_mcp_server_allowlist(servers, "agents.spawn.mcp_servers")) + .transpose()?; + Ok(AgentSpawnConfig { mcp_servers }) +} + +fn normalize_mcp_server_allowlist( + servers: &[String], + field_label: &str, +) -> std::io::Result> { + let mut normalized = Vec::with_capacity(servers.len()); + let mut seen = std::collections::BTreeSet::new(); + + for server in servers { + let server = server.trim(); + if server.is_empty() { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + format!("{field_label} cannot contain blank MCP server names"), + )); + } + if seen.insert(server.to_string()) { + normalized.push(server.to_string()); + } + } + + Ok(normalized) +} + fn multi_agent_v2_toml_config(features: Option<&FeaturesToml>) -> Option<&MultiAgentV2ConfigToml> { match features?.multi_agent_v2.as_ref()? { FeatureToml::Enabled(_) => None, @@ -1839,6 +1894,11 @@ impl Config { "agents.job_max_runtime_seconds must fit within a 64-bit signed integer", )); } + let agent_spawn = resolve_agent_spawn_config( + cfg.agents + .as_ref() + .and_then(|agents| agents.spawn.as_ref()), + )?; let background_terminal_max_timeout = cfg .background_terminal_max_timeout .unwrap_or(DEFAULT_MAX_BACKGROUND_TERMINAL_TIMEOUT_MS) @@ -2151,6 +2211,7 @@ impl Config { env!("CARGO_PKG_VERSION"), ), mcp_servers, + mcp_server_allowlist: None, // The config.toml omits "_mode" because it's a config file. However, "_mode" // is important in code to differentiate the mode from the store implementation. mcp_oauth_credentials_store_mode: resolve_mcp_oauth_credentials_store_mode( @@ -2177,6 +2238,7 @@ impl Config { tool_output_token_limit: cfg.tool_output_token_limit, agent_max_threads, agent_max_depth, + agent_spawn, agent_roles, memories: cfg.memories.unwrap_or_default().into(), agent_job_max_runtime_seconds, diff --git a/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs b/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs index 523b1ed357..c09b8dbc4d 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs @@ -65,6 +65,7 @@ impl ToolHandler for Handler { role_name, args.model.as_deref(), args.reasoning_effort, + args.mcp_servers.as_deref(), )?; } else { apply_requested_spawn_agent_model_overrides( @@ -78,6 +79,12 @@ impl ToolHandler for Handler { apply_role_to_config(&mut config, role_name) .await .map_err(FunctionCallError::RespondToModel)?; + apply_spawn_agent_capability_overrides( + &session, + &mut config, + args.mcp_servers, + ) + .await?; } apply_spawn_agent_runtime_overrides(&mut config, turn.as_ref())?; apply_spawn_agent_overrides(&mut config, child_depth); @@ -182,6 +189,7 @@ struct SpawnAgentArgs { agent_type: Option, model: Option, reasoning_effort: Option, + mcp_servers: Option>, #[serde(default)] fork_context: bool, } diff --git a/codex-rs/core/src/tools/handlers/multi_agents_common.rs b/codex-rs/core/src/tools/handlers/multi_agents_common.rs index 396496a353..5547c518e6 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_common.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_common.rs @@ -7,6 +7,7 @@ use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; use codex_features::Feature; +use codex_mcp::effective_mcp_servers; use codex_models_manager::manager::RefreshStrategy; use codex_protocol::AgentPath; use codex_protocol::ThreadId; @@ -23,6 +24,7 @@ use codex_protocol::protocol::SubAgentSource; use codex_protocol::user_input::UserInput; use serde::Serialize; use serde_json::Value as JsonValue; +use std::collections::BTreeSet; use std::collections::HashMap; /// Minimum wait timeout to prevent tight polling loops from burning CPU. @@ -240,15 +242,99 @@ pub(crate) fn reject_full_fork_spawn_overrides( agent_type: Option<&str>, model: Option<&str>, reasoning_effort: Option, + mcp_servers: Option<&[String]>, ) -> Result<(), FunctionCallError> { - if agent_type.is_some() || model.is_some() || reasoning_effort.is_some() { + if agent_type.is_some() + || model.is_some() + || reasoning_effort.is_some() + || mcp_servers.is_some() + { return Err(FunctionCallError::RespondToModel( - "Full-history forked agents inherit the parent agent type, model, and reasoning effort; omit agent_type, model, and reasoning_effort, or spawn without fork_context/fork_turns=all.".to_string(), + "Full-history forked agents inherit the parent agent type, model, reasoning effort, and MCP servers; omit agent_type, model, reasoning_effort, and mcp_servers, or spawn without fork_context/fork_turns=all.".to_string(), )); } Ok(()) } +pub(crate) async fn apply_spawn_agent_capability_overrides( + session: &Session, + config: &mut Config, + requested_mcp_servers: Option>, +) -> Result<(), FunctionCallError> { + let mcp_server_allowlist = match requested_mcp_servers { + Some(servers) => Some(normalize_spawn_mcp_server_allowlist( + &servers, + "mcp_servers", + )?), + None => config.agent_spawn.mcp_servers.clone(), + }; + + if let Some(allowlist) = mcp_server_allowlist.as_ref() { + validate_spawn_mcp_server_allowlist(session, config, allowlist).await?; + } + + config.mcp_server_allowlist = mcp_server_allowlist; + Ok(()) +} + +fn normalize_spawn_mcp_server_allowlist( + servers: &[String], + field_label: &str, +) -> Result, FunctionCallError> { + let mut normalized = Vec::with_capacity(servers.len()); + let mut seen = BTreeSet::new(); + + for server in servers { + let server = server.trim(); + if server.is_empty() { + return Err(FunctionCallError::RespondToModel(format!( + "{field_label} cannot contain blank MCP server names" + ))); + } + if seen.insert(server.to_string()) { + normalized.push(server.to_string()); + } + } + + Ok(normalized) +} + +async fn validate_spawn_mcp_server_allowlist( + session: &Session, + config: &Config, + allowlist: &[String], +) -> Result<(), FunctionCallError> { + let auth = session.services.auth_manager.auth().await; + let mut unfiltered_config = config.clone(); + unfiltered_config.mcp_server_allowlist = None; + let mcp_config = unfiltered_config + .to_mcp_config(session.services.plugins_manager.as_ref()) + .await; + let available_servers = effective_mcp_servers(&mcp_config, auth.as_ref()) + .keys() + .cloned() + .collect::>(); + let unknown_servers = allowlist + .iter() + .filter(|server| !available_servers.contains(*server)) + .cloned() + .collect::>(); + + if unknown_servers.is_empty() { + return Ok(()); + } + + let available = if available_servers.is_empty() { + "none".to_string() + } else { + available_servers.into_iter().collect::>().join(", ") + }; + Err(FunctionCallError::RespondToModel(format!( + "Unknown MCP server(s) for spawned agent: {}. Available MCP servers: {available}", + unknown_servers.join(", ") + ))) +} + /// Copies runtime-only turn state onto a child config before it is handed to `AgentControl`. /// /// These values are chosen by the live turn rather than persisted config, so leaving them stale diff --git a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs index 3a525e69eb..2f1c2996c9 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs @@ -17,6 +17,8 @@ use crate::tools::handlers::multi_agents_v2::SendMessageHandler as SendMessageHa use crate::tools::handlers::multi_agents_v2::SpawnAgentHandler as SpawnAgentHandlerV2; use crate::tools::handlers::multi_agents_v2::WaitAgentHandler as WaitAgentHandlerV2; use crate::turn_diff_tracker::TurnDiffTracker; +use codex_config::types::McpServerConfig; +use codex_config::types::McpServerTransportConfig; use codex_config::types::ShellEnvironmentPolicy; use codex_features::Feature; use codex_login::AuthManager; @@ -92,6 +94,30 @@ fn thread_manager() -> ThreadManager { ) } +fn test_mcp_server(url: &str) -> McpServerConfig { + McpServerConfig { + transport: McpServerTransportConfig::StreamableHttp { + url: url.to_string(), + bearer_token_env_var: None, + http_headers: None, + env_http_headers: None, + }, + experimental_environment: None, + enabled: false, + required: false, + supports_parallel_tool_calls: false, + disabled_reason: None, + startup_timeout_sec: None, + tool_timeout_sec: None, + default_tools_approval_mode: None, + enabled_tools: None, + disabled_tools: None, + scopes: None, + oauth_resource: None, + tools: HashMap::new(), + } +} + async fn install_role_with_model_override(turn: &mut TurnContext) -> String { let role_name = "fork-context-role".to_string(); tokio::fs::create_dir_all(&turn.config.codex_home) @@ -402,6 +428,304 @@ async fn spawn_agent_uses_explorer_role_and_preserves_approval_policy() { assert_eq!(snapshot.model_provider_id, "ollama"); } +#[tokio::test] +async fn spawn_agent_applies_per_call_mcp_settings() { + #[derive(Debug, Deserialize)] + struct SpawnAgentResult { + agent_id: String, + nickname: Option, + } + + let (mut session, mut turn) = make_session_and_context().await; + let manager = thread_manager(); + session.services.agent_control = manager.agent_control(); + let mut config = (*turn.config).clone(); + let mut mcp_servers = config.mcp_servers.get().clone(); + mcp_servers.insert( + "docs".to_string(), + test_mcp_server("https://docs.example/mcp"), + ); + mcp_servers.insert( + "linear".to_string(), + test_mcp_server("https://linear.example/mcp"), + ); + config + .mcp_servers + .set(mcp_servers) + .expect("test config should allow MCP updates"); + turn.config = Arc::new(config); + + let output = SpawnAgentHandler + .handle(invocation( + Arc::new(session), + Arc::new(turn), + "spawn_agent", + function_payload(json!({ + "message": "inspect this repo", + "mcp_servers": [" docs ", "docs"] + })), + )) + .await + .expect("spawn_agent should succeed"); + let (content, _) = expect_text_output(output); + let result: SpawnAgentResult = + serde_json::from_str(&content).expect("spawn_agent result should be json"); + assert!(result.nickname.is_some()); + + let child_config = manager + .get_thread(parse_agent_id(&result.agent_id)) + .await + .expect("spawned agent thread should exist") + .codex + .session + .get_config() + .await; + assert_eq!( + child_config.mcp_server_allowlist, + Some(vec!["docs".to_string()]) + ); +} + +#[tokio::test] +async fn spawn_agent_uses_role_spawn_defaults_and_allows_per_call_override() { + #[derive(Debug, Deserialize)] + struct SpawnAgentResult { + agent_id: String, + } + + let (mut session, mut turn) = make_session_and_context().await; + let manager = thread_manager(); + session.services.agent_control = manager.agent_control(); + + let role_name = "scoped".to_string(); + tokio::fs::create_dir_all(&turn.config.codex_home) + .await + .expect("codex home should be created"); + let role_config_path = turn.config.codex_home.as_path().join("scoped.toml"); + tokio::fs::write( + &role_config_path, + r#"[mcp_servers.docs] +url = "https://docs.example/mcp" +enabled = false + +[mcp_servers.linear] +url = "https://linear.example/mcp" +enabled = false + +[agents.spawn] +mcp_servers = ["linear"] +"#, + ) + .await + .expect("role config should be written"); + + let mut config = (*turn.config).clone(); + let mut mcp_servers = config.mcp_servers.get().clone(); + mcp_servers.insert( + "docs".to_string(), + test_mcp_server("https://docs.example/mcp"), + ); + mcp_servers.insert( + "linear".to_string(), + test_mcp_server("https://linear.example/mcp"), + ); + config + .mcp_servers + .set(mcp_servers) + .expect("test config should allow MCP updates"); + config.agent_roles.insert( + role_name.clone(), + AgentRoleConfig { + description: Some("Scoped tools".to_string()), + config_file: Some(role_config_path), + nickname_candidates: None, + }, + ); + turn.config = Arc::new(config); + + let output = SpawnAgentHandler + .handle(invocation( + Arc::new(session), + Arc::new(turn), + "spawn_agent", + function_payload(json!({ + "message": "inspect this repo", + "agent_type": role_name, + "mcp_servers": ["docs"] + })), + )) + .await + .expect("spawn_agent should succeed"); + let (content, _) = expect_text_output(output); + let result: SpawnAgentResult = + serde_json::from_str(&content).expect("spawn_agent result should be json"); + + let child_config = manager + .get_thread(parse_agent_id(&result.agent_id)) + .await + .expect("spawned agent thread should exist") + .codex + .session + .get_config() + .await; + assert_eq!( + child_config.agent_spawn.mcp_servers, + Some(vec!["linear".to_string()]) + ); + assert_eq!( + child_config.mcp_server_allowlist, + Some(vec!["docs".to_string()]) + ); +} + +#[tokio::test] +async fn spawn_agent_applies_role_spawn_defaults_without_per_call_overrides() { + #[derive(Debug, Deserialize)] + struct SpawnAgentResult { + agent_id: String, + } + + let (mut session, mut turn) = make_session_and_context().await; + let manager = thread_manager(); + session.services.agent_control = manager.agent_control(); + + let role_name = "scoped_defaults".to_string(); + tokio::fs::create_dir_all(&turn.config.codex_home) + .await + .expect("codex home should be created"); + let role_config_path = turn + .config + .codex_home + .as_path() + .join("scoped-defaults.toml"); + tokio::fs::write( + &role_config_path, + r#"[mcp_servers.linear] +url = "https://linear.example/mcp" +enabled = false + +[agents.spawn] +mcp_servers = ["linear"] +"#, + ) + .await + .expect("role config should be written"); + + let mut config = (*turn.config).clone(); + let mut mcp_servers = config.mcp_servers.get().clone(); + mcp_servers.insert( + "linear".to_string(), + test_mcp_server("https://linear.example/mcp"), + ); + config + .mcp_servers + .set(mcp_servers) + .expect("test config should allow MCP updates"); + config.agent_roles.insert( + role_name.clone(), + AgentRoleConfig { + description: Some("Scoped defaults".to_string()), + config_file: Some(role_config_path), + nickname_candidates: None, + }, + ); + turn.config = Arc::new(config); + + let output = SpawnAgentHandler + .handle(invocation( + Arc::new(session), + Arc::new(turn), + "spawn_agent", + function_payload(json!({ + "message": "inspect this repo", + "agent_type": role_name + })), + )) + .await + .expect("spawn_agent should succeed"); + let (content, _) = expect_text_output(output); + let result: SpawnAgentResult = + serde_json::from_str(&content).expect("spawn_agent result should be json"); + + let child_config = manager + .get_thread(parse_agent_id(&result.agent_id)) + .await + .expect("spawned agent thread should exist") + .codex + .session + .get_config() + .await; + assert_eq!( + child_config.mcp_server_allowlist, + Some(vec!["linear".to_string()]) + ); +} + +#[tokio::test] +async fn spawn_agent_rejects_unknown_mcp_server() { + let (mut session, mut turn) = make_session_and_context().await; + let manager = thread_manager(); + session.services.agent_control = manager.agent_control(); + let mut config = (*turn.config).clone(); + let mut mcp_servers = config.mcp_servers.get().clone(); + mcp_servers.insert( + "docs".to_string(), + test_mcp_server("https://docs.example/mcp"), + ); + config + .mcp_servers + .set(mcp_servers) + .expect("test config should allow MCP updates"); + turn.config = Arc::new(config); + + let err = SpawnAgentHandler + .handle(invocation( + Arc::new(session), + Arc::new(turn), + "spawn_agent", + function_payload(json!({ + "message": "inspect this repo", + "mcp_servers": ["missing"] + })), + )) + .await + .expect_err("unknown MCP server should be rejected"); + + assert_eq!( + err, + FunctionCallError::RespondToModel( + "Unknown MCP server(s) for spawned agent: missing. Available MCP servers: docs" + .to_string() + ) + ); +} + +#[tokio::test] +async fn spawn_agent_fork_context_rejects_capability_overrides() { + let (session, turn) = make_session_and_context().await; + + let err = SpawnAgentHandler + .handle(invocation( + Arc::new(session), + Arc::new(turn), + "spawn_agent", + function_payload(json!({ + "message": "inspect this repo", + "fork_context": true, + "mcp_servers": [] + })), + )) + .await + .expect_err("fork_context should reject capability overrides"); + + assert_eq!( + err, + FunctionCallError::RespondToModel( + "Full-history forked agents inherit the parent agent type, model, reasoning effort, and MCP servers; omit agent_type, model, reasoning_effort, and mcp_servers, or spawn without fork_context/fork_turns=all.".to_string(), + ) + ); +} + #[tokio::test] async fn spawn_agent_fork_context_rejects_agent_type_override() { let (mut session, mut turn) = make_session_and_context().await; @@ -430,7 +754,7 @@ async fn spawn_agent_fork_context_rejects_agent_type_override() { assert_eq!( err, FunctionCallError::RespondToModel( - "Full-history forked agents inherit the parent agent type, model, and reasoning effort; omit agent_type, model, and reasoning_effort, or spawn without fork_context/fork_turns=all.".to_string(), + "Full-history forked agents inherit the parent agent type, model, reasoning effort, and MCP servers; omit agent_type, model, reasoning_effort, and mcp_servers, or spawn without fork_context/fork_turns=all.".to_string(), ) ); } @@ -464,7 +788,7 @@ async fn spawn_agent_fork_context_rejects_child_model_overrides() { assert_eq!( err, FunctionCallError::RespondToModel( - "Full-history forked agents inherit the parent agent type, model, and reasoning effort; omit agent_type, model, and reasoning_effort, or spawn without fork_context/fork_turns=all.".to_string(), + "Full-history forked agents inherit the parent agent type, model, reasoning effort, and MCP servers; omit agent_type, model, reasoning_effort, and mcp_servers, or spawn without fork_context/fork_turns=all.".to_string(), ) ); } @@ -508,7 +832,7 @@ async fn multi_agent_v2_spawn_fork_turns_all_rejects_agent_type_override() { assert_eq!( err, FunctionCallError::RespondToModel( - "Full-history forked agents inherit the parent agent type, model, and reasoning effort; omit agent_type, model, and reasoning_effort, or spawn without fork_context/fork_turns=all.".to_string(), + "Full-history forked agents inherit the parent agent type, model, reasoning effort, and MCP servers; omit agent_type, model, reasoning_effort, and mcp_servers, or spawn without fork_context/fork_turns=all.".to_string(), ) ); } @@ -549,7 +873,7 @@ async fn multi_agent_v2_spawn_fork_turns_rejects_child_model_overrides() { assert_eq!( err, FunctionCallError::RespondToModel( - "Full-history forked agents inherit the parent agent type, model, and reasoning effort; omit agent_type, model, and reasoning_effort, or spawn without fork_context/fork_turns=all.".to_string(), + "Full-history forked agents inherit the parent agent type, model, reasoning effort, and MCP servers; omit agent_type, model, reasoning_effort, and mcp_servers, or spawn without fork_context/fork_turns=all.".to_string(), ) ); } @@ -829,6 +1153,68 @@ async fn multi_agent_v2_spawn_returns_path_and_send_message_accepts_relative_pat })); } +#[tokio::test] +async fn multi_agent_v2_spawn_applies_per_call_mcp_settings() { + let (mut session, mut turn) = make_session_and_context().await; + let manager = thread_manager(); + let root = manager + .start_thread((*turn.config).clone()) + .await + .expect("root thread should start"); + session.services.agent_control = manager.agent_control(); + session.conversation_id = root.thread_id; + let mut config = (*turn.config).clone(); + config + .features + .enable(Feature::MultiAgentV2) + .expect("test config should allow feature update"); + let mut mcp_servers = config.mcp_servers.get().clone(); + mcp_servers.insert( + "docs".to_string(), + test_mcp_server("https://docs.example/mcp"), + ); + config + .mcp_servers + .set(mcp_servers) + .expect("test config should allow MCP updates"); + turn.config = Arc::new(config); + let session = Arc::new(session); + let turn = Arc::new(turn); + + SpawnAgentHandlerV2 + .handle(invocation( + session.clone(), + turn, + "spawn_agent", + function_payload(json!({ + "message": "inspect this repo", + "task_name": "test_process", + "mcp_servers": ["docs"] + })), + )) + .await + .expect("spawn_agent should succeed"); + + let child_thread_id = session + .services + .agent_control + .resolve_agent_reference(root.thread_id, &SessionSource::Cli, "test_process") + .await + .expect("relative path should resolve"); + let child_config = manager + .get_thread(child_thread_id) + .await + .expect("child thread should exist") + .codex + .session + .get_config() + .await; + assert_eq!( + child_config.mcp_server_allowlist, + Some(vec!["docs".to_string()]) + ); +} + #[tokio::test] async fn multi_agent_v2_spawn_rejects_legacy_fork_context() { let (mut session, mut turn) = make_session_and_context().await; diff --git a/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs b/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs index 03287b20a8..82334dfaac 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs @@ -75,6 +75,7 @@ impl ToolHandler for Handler { role_name, args.model.as_deref(), args.reasoning_effort, + args.mcp_servers.as_deref(), )?; } else { apply_requested_spawn_agent_model_overrides( @@ -88,6 +89,12 @@ impl ToolHandler for Handler { apply_role_to_config(&mut config, role_name) .await .map_err(FunctionCallError::RespondToModel)?; + apply_spawn_agent_capability_overrides( + &session, + &mut config, + args.mcp_servers, + ) + .await?; } apply_spawn_agent_runtime_overrides(&mut config, turn.as_ref())?; apply_spawn_agent_overrides(&mut config, child_depth); @@ -234,6 +241,7 @@ struct SpawnAgentArgs { agent_type: Option, model: Option, reasoning_effort: Option, + mcp_servers: Option>, fork_turns: Option, fork_context: Option, } diff --git a/codex-rs/tools/src/agent_tool.rs b/codex-rs/tools/src/agent_tool.rs index aa1699a4e0..8bb97e846a 100644 --- a/codex-rs/tools/src/agent_tool.rs +++ b/codex-rs/tools/src/agent_tool.rs @@ -538,6 +538,16 @@ fn spawn_agent_common_properties_v1(agent_type_description: &str) -> BTreeMap BTreeMap