Simplify MCP runtime environment selection

This commit is contained in:
starr-openai
2026-05-18 19:06:37 -07:00
parent 4fdb044248
commit d265ea2cf3
8 changed files with 29 additions and 96 deletions

View File

@@ -209,11 +209,6 @@ impl McpRequestProcessor {
// executor-backed stdio MCPs whose config omits `cwd`.
let runtime_environment = McpRuntimeEnvironment::new(
environment_manager.default_or_local_environment(),
if environment_manager.has_local_environment() {
codex_mcp::LocalStdioAvailability::Enabled
} else {
codex_mcp::LocalStdioAvailability::Disabled
},
config.cwd.to_path_buf(),
);
@@ -374,11 +369,6 @@ impl McpRequestProcessor {
// used only by executor-backed stdio MCPs whose config omits `cwd`.
let runtime_environment = McpRuntimeEnvironment::new(
environment_manager.default_or_local_environment(),
if environment_manager.has_local_environment() {
codex_mcp::LocalStdioAvailability::Enabled
} else {
codex_mcp::LocalStdioAvailability::Disabled
},
config.cwd.to_path_buf(),
);
let request_id = request_id.clone();

View File

@@ -10,7 +10,6 @@ use crate::elicitation::elicitation_is_rejected_by_policy;
use crate::rmcp_client::AsyncManagedClient;
use crate::rmcp_client::ManagedClient;
use crate::rmcp_client::StartupOutcomeError;
use crate::runtime::LocalStdioAvailability;
use crate::server::McpServerOrigin;
use crate::tools::ToolFilter;
use crate::tools::ToolInfo;
@@ -916,11 +915,7 @@ async fn no_local_runtime_skips_local_stdio_but_keeps_local_http_server() {
String::new(),
tx_event,
PermissionProfile::default(),
McpRuntimeEnvironment::new(
/*environment*/ None,
LocalStdioAvailability::Disabled,
PathBuf::from("/tmp"),
),
McpRuntimeEnvironment::new(/*environment*/ None, PathBuf::from("/tmp")),
codex_home.path().to_path_buf(),
CodexAppsToolsCacheKey {
account_id: None,

View File

@@ -3,7 +3,6 @@ pub use elicitation::ElicitationReviewRequest;
pub use elicitation::ElicitationReviewer;
pub use elicitation::ElicitationReviewerHandle;
pub use rmcp_client::MCP_SANDBOX_STATE_META_CAPABILITY;
pub use runtime::LocalStdioAvailability;
pub use runtime::McpRuntimeEnvironment;
pub use runtime::SandboxState;
pub use tools::ToolInfo;

View File

@@ -31,34 +31,21 @@ pub struct SandboxState {
/// Runtime placement information used when starting MCP server transports.
///
/// `McpConfig` describes what servers exist. This value describes which
/// selected/default environment remote MCP servers should use plus whether
/// local stdio MCP startup is allowed for the current caller. Keep it explicit
/// at manager construction time so status/snapshot paths and real sessions
/// make the same placement decision. `fallback_cwd` is not a per-server
/// override; it is used when a stdio server omits `cwd` and the launcher needs
/// a concrete process working directory.
/// selected/default environment MCP servers should use for the current caller.
/// Keep it explicit at manager construction time so status/snapshot paths and
/// real sessions make the same placement decision. `fallback_cwd` is not a
/// per-server override; it is used when a stdio server omits `cwd` and the
/// launcher needs a concrete process working directory.
#[derive(Clone)]
pub struct McpRuntimeEnvironment {
environment: Option<Arc<Environment>>,
local_stdio_availability: LocalStdioAvailability,
fallback_cwd: PathBuf,
}
#[derive(Clone, Copy)]
pub enum LocalStdioAvailability {
Enabled,
Disabled,
}
impl McpRuntimeEnvironment {
pub fn new(
environment: Option<Arc<Environment>>,
local_stdio_availability: LocalStdioAvailability,
fallback_cwd: PathBuf,
) -> Self {
pub fn new(environment: Option<Arc<Environment>>, fallback_cwd: PathBuf) -> Self {
Self {
environment,
local_stdio_availability,
fallback_cwd,
}
}
@@ -78,13 +65,15 @@ impl McpRuntimeEnvironment {
) -> Option<String> {
match config.experimental_environment.as_deref() {
None | Some("local") => {
if matches!(
self.local_stdio_availability,
LocalStdioAvailability::Disabled
) && matches!(
config.transport,
codex_config::McpServerTransportConfig::Stdio { .. }
) {
if !self
.environment
.as_ref()
.is_some_and(|environment| !environment.is_remote())
&& matches!(
config.transport,
codex_config::McpServerTransportConfig::Stdio { .. }
)
{
Some(format!(
"local stdio MCP server `{server_name}` requires a local environment"
))
@@ -92,7 +81,7 @@ impl McpRuntimeEnvironment {
None
}
}
Some("remote") => match self.environment() {
Some("remote") => match self.environment.as_ref() {
Some(environment) if environment.is_remote() => None,
_ => Some(format!(
"remote MCP server `{server_name}` requires a remote environment"
@@ -162,11 +151,8 @@ mod tests {
#[test]
fn local_stdio_requires_local_stdio_availability() {
let runtime_environment = McpRuntimeEnvironment::new(
/*environment*/ None,
LocalStdioAvailability::Disabled,
PathBuf::from("/tmp"),
);
let runtime_environment =
McpRuntimeEnvironment::new(/*environment*/ None, PathBuf::from("/tmp"));
assert_eq!(
runtime_environment.startup_unavailable_reason("stdio", &stdio_server(None)),
@@ -176,11 +162,8 @@ mod tests {
#[test]
fn local_http_does_not_require_local_stdio_availability() {
let runtime_environment = McpRuntimeEnvironment::new(
/*environment*/ None,
LocalStdioAvailability::Disabled,
PathBuf::from("/tmp"),
);
let runtime_environment =
McpRuntimeEnvironment::new(/*environment*/ None, PathBuf::from("/tmp"));
assert_eq!(
runtime_environment.startup_unavailable_reason("http", &http_server(None)),
@@ -190,11 +173,8 @@ mod tests {
#[test]
fn remote_stdio_requires_remote_environment() {
let runtime_environment = McpRuntimeEnvironment::new(
/*environment*/ None,
LocalStdioAvailability::Enabled,
PathBuf::from("/tmp"),
);
let runtime_environment =
McpRuntimeEnvironment::new(/*environment*/ None, PathBuf::from("/tmp"));
assert_eq!(
runtime_environment.startup_unavailable_reason("stdio", &stdio_server(Some("remote"))),
@@ -208,11 +188,8 @@ mod tests {
Environment::create_for_tests(Some("ws://127.0.0.1:8765".to_string()))
.expect("remote environment"),
);
let runtime_environment = McpRuntimeEnvironment::new(
Some(environment),
LocalStdioAvailability::Disabled,
PathBuf::from("/tmp"),
);
let runtime_environment =
McpRuntimeEnvironment::new(Some(environment), PathBuf::from("/tmp"));
assert_eq!(
runtime_environment.startup_unavailable_reason("stdio", &stdio_server(Some("remote"))),

View File

@@ -272,11 +272,6 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_environment_manager(
PermissionProfile::default(),
McpRuntimeEnvironment::new(
environment_manager.default_or_local_environment(),
if environment_manager.has_local_environment() {
codex_mcp::LocalStdioAvailability::Enabled
} else {
codex_mcp::LocalStdioAvailability::Disabled
},
config.cwd.to_path_buf(),
),
config.codex_home.to_path_buf(),

View File

@@ -1245,14 +1245,10 @@ async fn install_host_owned_codex_apps_manager(session: &Session, turn_context:
turn_context.sub_id.clone(),
session.get_tx_event(),
turn_context.permission_profile(),
codex_mcp::McpRuntimeEnvironment::new(
Some(environment),
codex_mcp::LocalStdioAvailability::Enabled,
{
#[allow(deprecated)]
turn_context.cwd.to_path_buf()
},
),
codex_mcp::McpRuntimeEnvironment::new(Some(environment), {
#[allow(deprecated)]
turn_context.cwd.to_path_buf()
}),
turn_context.config.codex_home.to_path_buf(),
codex_mcp::codex_apps_tools_cache_key(auth.as_ref()),
/*host_owned_codex_apps_enabled*/ true,

View File

@@ -289,23 +289,15 @@ impl Session {
host_owned_codex_apps_enabled(&mcp_config, auth.as_ref());
let auth_statuses =
compute_auth_statuses(mcp_servers.iter(), store_mode, auth.as_ref()).await;
let local_stdio_availability = if self.services.environment_manager.has_local_environment()
{
codex_mcp::LocalStdioAvailability::Enabled
} else {
codex_mcp::LocalStdioAvailability::Disabled
};
let mcp_runtime_environment = match turn_context.environments.primary() {
Some(turn_environment) => McpRuntimeEnvironment::new(
Some(Arc::clone(&turn_environment.environment)),
local_stdio_availability,
turn_environment.cwd.to_path_buf(),
),
None => McpRuntimeEnvironment::new(
self.services
.environment_manager
.default_or_local_environment(),
local_stdio_availability,
#[allow(deprecated)]
turn_context.cwd.to_path_buf(),
),

View File

@@ -1046,24 +1046,13 @@ impl Session {
})?
.primary()
.cloned();
let local_stdio_availability = if sess
.services
.environment_manager
.has_local_environment()
{
codex_mcp::LocalStdioAvailability::Enabled
} else {
codex_mcp::LocalStdioAvailability::Disabled
};
let mcp_runtime_environment = match turn_environment {
Some(turn_environment) => McpRuntimeEnvironment::new(
Some(Arc::clone(&turn_environment.environment)),
local_stdio_availability,
turn_environment.cwd.to_path_buf(),
),
None => McpRuntimeEnvironment::new(
sess.services.environment_manager.default_or_local_environment(),
local_stdio_availability,
session_configuration.cwd.to_path_buf(),
),
};