mirror of
https://github.com/openai/codex.git
synced 2026-11-12 22:42:36 +00:00
Route MCP servers through explicit environments (#23583)
## Summary - route each configured MCP server through an explicit per-server `environment_id` instead of a manager-wide remote toggle - default omitted `environment_id` to `local`, resolve named ids through `EnvironmentManager`, and fail only the affected MCP server when an explicit id is unknown - keep local stdio on the existing local launcher path for now, while named-environment stdio uses the selected environment backend and requires an absolute `cwd` - allow local HTTP MCP servers to keep using the ambient HTTP client when no local `Environment` is configured; named-environment HTTP MCPs use that environment's HTTP client ## Validation - devbox Bazel build: `bazel build --bes_backend= --bes_results_url= //codex-rs/cli:codex //codex-rs/rmcp-client:test_stdio_server //codex-rs/rmcp-client:test_streamable_http_server` - devbox app-server config matrix with real `config.toml` / `environments.toml` files covering omitted local, explicit local, omitted local under remote default, explicit remote stdio, local HTTP without local env, explicit remote HTTP, local stdio without local env, unknown explicit env, and remote stdio without `cwd`
This commit is contained in:
1 parent
97b390fbd4
commit
298e5cfce1
36 files changed
+463
-431
No files matched your search
@@ -242,7 +242,7 @@ async fn run_code_mode_turn_with_rmcp_config(
|
||||
env_vars: Vec::new(),
|
||||
cwd: None,
|
||||
},
|
||||
experimental_environment: None,
|
||||
environment_id: "local".to_string(),
|
||||
enabled: true,
|
||||
required: false,
|
||||
supports_parallel_tool_calls: false,
|
||||
|
||||
@@ -182,7 +182,7 @@ fn insert_rmcp_test_server(config: &mut Config, command: String, approval_mode:
|
||||
env_vars: Vec::new(),
|
||||
cwd: None,
|
||||
},
|
||||
experimental_environment: None,
|
||||
environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(),
|
||||
enabled: true,
|
||||
required: false,
|
||||
supports_parallel_tool_calls: false,
|
||||
|
||||
@@ -139,11 +139,13 @@ enum McpCallEvent {
|
||||
|
||||
const REMOTE_MCP_ENVIRONMENT: &str = "remote";
|
||||
|
||||
fn remote_aware_experimental_environment() -> Option<String> {
|
||||
fn remote_aware_environment_id() -> String {
|
||||
// These tests run locally in normal CI and against the Docker-backed
|
||||
// executor in full-ci. Match that shared test environment instead of
|
||||
// parameterizing each stdio MCP test with its own local/remote cases.
|
||||
std::env::var_os(remote_env_env_var()).map(|_| REMOTE_MCP_ENVIRONMENT.to_string())
|
||||
std::env::var_os(remote_env_env_var())
|
||||
.map(|_| REMOTE_MCP_ENVIRONMENT.to_string())
|
||||
.unwrap_or_else(|| codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string())
|
||||
}
|
||||
|
||||
/// Returns the stdio MCP test server command path for the active test placement.
|
||||
@@ -285,13 +287,22 @@ async fn wait_for_mcp_server(fixture: &TestCodex, server_name: &str) -> anyhow::
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[derive(Default)]
|
||||
struct TestMcpServerOptions {
|
||||
experimental_environment: Option<String>,
|
||||
environment_id: String,
|
||||
supports_parallel_tool_calls: bool,
|
||||
tool_timeout_sec: Option<Duration>,
|
||||
}
|
||||
|
||||
impl Default for TestMcpServerOptions {
|
||||
fn default() -> Self {
|
||||
Self {
|
||||
environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(),
|
||||
supports_parallel_tool_calls: false,
|
||||
tool_timeout_sec: None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn stdio_transport(
|
||||
command: String,
|
||||
env: Option<HashMap<String, String>>,
|
||||
@@ -326,7 +337,7 @@ fn insert_mcp_server(
|
||||
server_name.to_string(),
|
||||
McpServerConfig {
|
||||
transport,
|
||||
experimental_environment: options.experimental_environment,
|
||||
environment_id: options.environment_id,
|
||||
enabled: true,
|
||||
required: false,
|
||||
supports_parallel_tool_calls: options.supports_parallel_tool_calls,
|
||||
@@ -480,7 +491,7 @@ async fn stdio_server_round_trip() -> anyhow::Result<()> {
|
||||
Vec::new(),
|
||||
),
|
||||
TestMcpServerOptions {
|
||||
experimental_environment: remote_aware_experimental_environment(),
|
||||
environment_id: remote_aware_environment_id(),
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
@@ -603,7 +614,7 @@ async fn stdio_server_uses_configured_cwd_before_runtime_fallback() -> anyhow::R
|
||||
Some(configured_cwd),
|
||||
),
|
||||
TestMcpServerOptions {
|
||||
experimental_environment: remote_aware_experimental_environment(),
|
||||
environment_id: remote_aware_environment_id(),
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
@@ -623,59 +634,6 @@ async fn stdio_server_uses_configured_cwd_before_runtime_fallback() -> anyhow::R
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
|
||||
#[serial(mcp_cwd)]
|
||||
async fn remote_stdio_server_uses_runtime_fallback_cwd_when_config_omits_cwd() -> anyhow::Result<()>
|
||||
{
|
||||
skip_if_no_network!(Ok(()));
|
||||
if std::env::var_os(remote_env_env_var()).is_none() {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let server = responses::start_mock_server().await;
|
||||
let server_name = "rmcp_fallback_cwd";
|
||||
let expected_cwd = Arc::new(Mutex::new(None::<PathBuf>));
|
||||
let expected_cwd_for_config = Arc::clone(&expected_cwd);
|
||||
let rmcp_test_server_bin = remote_aware_stdio_server_bin()?;
|
||||
|
||||
let fixture = test_codex()
|
||||
.with_config(move |config| {
|
||||
*expected_cwd_for_config
|
||||
.lock()
|
||||
.expect("expected cwd lock should not be poisoned") =
|
||||
Some(config.cwd.to_path_buf());
|
||||
insert_mcp_server(
|
||||
config,
|
||||
server_name,
|
||||
stdio_transport(
|
||||
rmcp_test_server_bin,
|
||||
Some(HashMap::from([(
|
||||
"MCP_TEST_VALUE".to_string(),
|
||||
"fallback-cwd".to_string(),
|
||||
)])),
|
||||
Vec::new(),
|
||||
),
|
||||
TestMcpServerOptions {
|
||||
experimental_environment: remote_aware_experimental_environment(),
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
})
|
||||
.build_with_remote_env(&server)
|
||||
.await?;
|
||||
|
||||
let expected_cwd = expected_cwd
|
||||
.lock()
|
||||
.expect("expected cwd lock should not be poisoned")
|
||||
.clone()
|
||||
.expect("test config should record runtime fallback cwd");
|
||||
let structured = call_cwd_tool(&server, &fixture, server_name, "call-fallback-cwd").await?;
|
||||
|
||||
assert_cwd_tool_output(&structured, &expected_cwd);
|
||||
server.verify().await;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
|
||||
#[serial(mcp_cwd)]
|
||||
@@ -775,7 +733,7 @@ async fn stdio_mcp_tool_call_includes_sandbox_state_meta() -> anyhow::Result<()>
|
||||
server_name,
|
||||
stdio_transport(rmcp_test_server_bin, /*env*/ None, Vec::new()),
|
||||
TestMcpServerOptions {
|
||||
experimental_environment: remote_aware_experimental_environment(),
|
||||
environment_id: remote_aware_environment_id(),
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
@@ -872,7 +830,7 @@ async fn stdio_mcp_parallel_tool_calls_default_false_runs_serially() -> anyhow::
|
||||
server_name,
|
||||
stdio_transport(rmcp_test_server_bin, /*env*/ None, Vec::new()),
|
||||
TestMcpServerOptions {
|
||||
experimental_environment: remote_aware_experimental_environment(),
|
||||
environment_id: remote_aware_environment_id(),
|
||||
tool_timeout_sec: Some(Duration::from_secs(2)),
|
||||
..Default::default()
|
||||
},
|
||||
@@ -989,7 +947,7 @@ async fn stdio_mcp_parallel_tool_calls_opt_in_runs_concurrently() -> anyhow::Res
|
||||
server_name,
|
||||
stdio_transport(rmcp_test_server_bin, /*env*/ None, Vec::new()),
|
||||
TestMcpServerOptions {
|
||||
experimental_environment: remote_aware_experimental_environment(),
|
||||
environment_id: remote_aware_environment_id(),
|
||||
supports_parallel_tool_calls: true,
|
||||
tool_timeout_sec: Some(Duration::from_secs(2)),
|
||||
},
|
||||
@@ -1071,7 +1029,7 @@ async fn stdio_image_responses_round_trip() -> anyhow::Result<()> {
|
||||
Vec::new(),
|
||||
),
|
||||
TestMcpServerOptions {
|
||||
experimental_environment: remote_aware_experimental_environment(),
|
||||
environment_id: remote_aware_environment_id(),
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
@@ -1204,7 +1162,7 @@ async fn stdio_image_responses_preserve_original_detail_metadata() -> anyhow::Re
|
||||
server_name,
|
||||
stdio_transport(rmcp_test_server_bin, /*env*/ None, Vec::new()),
|
||||
TestMcpServerOptions {
|
||||
experimental_environment: remote_aware_experimental_environment(),
|
||||
environment_id: remote_aware_environment_id(),
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
@@ -1340,7 +1298,7 @@ async fn stdio_image_responses_are_sanitized_for_text_only_model() -> anyhow::Re
|
||||
Vec::new(),
|
||||
),
|
||||
TestMcpServerOptions {
|
||||
experimental_environment: remote_aware_experimental_environment(),
|
||||
environment_id: remote_aware_environment_id(),
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
@@ -1442,7 +1400,7 @@ async fn stdio_server_propagates_whitelisted_env_vars() -> anyhow::Result<()> {
|
||||
vec!["MCP_TEST_VALUE".into()],
|
||||
),
|
||||
TestMcpServerOptions {
|
||||
experimental_environment: remote_aware_experimental_environment(),
|
||||
environment_id: remote_aware_environment_id(),
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
@@ -1560,7 +1518,7 @@ async fn stdio_server_propagates_explicit_local_env_var_source() -> anyhow::Resu
|
||||
}],
|
||||
),
|
||||
TestMcpServerOptions {
|
||||
experimental_environment: remote_aware_experimental_environment(),
|
||||
environment_id: remote_aware_environment_id(),
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
@@ -1652,7 +1610,7 @@ async fn remote_stdio_env_var_source_does_not_copy_local_env() -> anyhow::Result
|
||||
}],
|
||||
),
|
||||
TestMcpServerOptions {
|
||||
experimental_environment: remote_aware_experimental_environment(),
|
||||
environment_id: remote_aware_environment_id(),
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
@@ -1835,7 +1793,7 @@ async fn streamable_http_tool_call_round_trip() -> anyhow::Result<()> {
|
||||
env_http_headers: None,
|
||||
},
|
||||
TestMcpServerOptions {
|
||||
experimental_environment: remote_aware_experimental_environment(),
|
||||
environment_id: remote_aware_environment_id(),
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
@@ -2021,7 +1979,7 @@ async fn streamable_http_with_oauth_round_trip_impl() -> anyhow::Result<()> {
|
||||
env_http_headers: None,
|
||||
},
|
||||
TestMcpServerOptions {
|
||||
experimental_environment: remote_aware_experimental_environment(),
|
||||
environment_id: remote_aware_environment_id(),
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
|
||||
@@ -1012,7 +1012,7 @@ async fn tool_search_indexes_only_enabled_non_app_mcp_tools() -> Result<()> {
|
||||
env_vars: Vec::new(),
|
||||
cwd: None,
|
||||
},
|
||||
experimental_environment: None,
|
||||
environment_id: "local".to_string(),
|
||||
enabled: true,
|
||||
required: false,
|
||||
disabled_reason: None,
|
||||
@@ -1141,7 +1141,7 @@ async fn tool_search_surfaced_mcp_tool_errors_are_returned_to_model() -> Result<
|
||||
env_vars: Vec::new(),
|
||||
cwd: None,
|
||||
},
|
||||
experimental_environment: None,
|
||||
environment_id: "local".to_string(),
|
||||
enabled: true,
|
||||
required: false,
|
||||
disabled_reason: None,
|
||||
@@ -1288,7 +1288,7 @@ async fn tool_search_uses_non_app_mcp_server_instructions_as_namespace_descripti
|
||||
env_vars: Vec::new(),
|
||||
cwd: None,
|
||||
},
|
||||
experimental_environment: None,
|
||||
environment_id: "local".to_string(),
|
||||
enabled: true,
|
||||
required: false,
|
||||
disabled_reason: None,
|
||||
|
||||
@@ -376,7 +376,7 @@ async fn mcp_call_marks_thread_memory_mode_polluted_when_configured() -> Result<
|
||||
env_vars: Vec::new(),
|
||||
cwd: None,
|
||||
},
|
||||
experimental_environment: None,
|
||||
environment_id: "local".to_string(),
|
||||
enabled: true,
|
||||
required: false,
|
||||
supports_parallel_tool_calls: false,
|
||||
|
||||
@@ -387,7 +387,7 @@ async fn mcp_tool_call_output_exceeds_limit_truncated_for_model() -> Result<()>
|
||||
env_vars: Vec::new(),
|
||||
cwd: None,
|
||||
},
|
||||
experimental_environment: None,
|
||||
environment_id: "local".to_string(),
|
||||
enabled: true,
|
||||
required: false,
|
||||
supports_parallel_tool_calls: false,
|
||||
@@ -487,7 +487,7 @@ async fn mcp_image_output_preserves_image_and_no_text_summary() -> Result<()> {
|
||||
env_vars: Vec::new(),
|
||||
cwd: None,
|
||||
},
|
||||
experimental_environment: None,
|
||||
environment_id: "local".to_string(),
|
||||
enabled: true,
|
||||
required: false,
|
||||
supports_parallel_tool_calls: false,
|
||||
@@ -775,7 +775,7 @@ async fn mcp_tool_call_output_not_truncated_with_custom_limit() -> Result<()> {
|
||||
env_vars: Vec::new(),
|
||||
cwd: None,
|
||||
},
|
||||
experimental_environment: None,
|
||||
environment_id: "local".to_string(),
|
||||
enabled: true,
|
||||
required: false,
|
||||
supports_parallel_tool_calls: false,
|
||||
|
||||
Reference in new issue
Block a user