openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-09-05 15:18:41 +00:00
Code Issues Packages Projects Releases Wiki Activity

Restore remote stdio MCP cwd fallback

Browse Source
This commit is contained in:
pakrym-oai
2026-06-12 16:43:05 -07:00
parent 7ebd10783b
commit b37c62544c
4 changed files with 21 additions and 150 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
codex-rs/codex-mcp/src/rmcp_client.rs
View File

@@ -601,6 +601,18 @@ async fn make_rmcp_client(
env_vars,
cwd,
} => {
let cwd = if is_local_environment {
cwd
} else {
let cwd = cwd.unwrap_or_else(|| runtime_context.stdio_fallback_cwd());
if !cwd.is_absolute() {
return Err(StartupOutcomeError::from(anyhow!(
"remote stdio MCP server `{server_name}` requires an absolute cwd, got `{}`",
cwd.display()
)));
}
Some(cwd)
};
let command_os: OsString = command.into();
let args_os: Vec<OsString> = args.into_iter().map(Into::into).collect();
let env_os = env.map(|env| {
@@ -613,7 +625,7 @@ async fn make_rmcp_client(
// `ExecutorStdioServerLauncher` once the executor-backed path
// preserves `LocalStdioServerLauncher` semantics.
Arc::new(LocalStdioServerLauncher::new(
runtime_context.local_stdio_fallback_cwd(),
runtime_context.stdio_fallback_cwd(),
)) as Arc<dyn StdioServerLauncher>
} else {
let Some(environment) = resolved_environment.as_ref() else {

76
codex-rs/codex-mcp/src/runtime.rs
View File

@@ -32,27 +32,24 @@ pub struct SandboxState {
/// Runtime context used when resolving per-server MCP environments.
///
/// `McpConfig` describes what servers exist. This value carries the canonical
/// environment registry plus the local stdio fallback cwd used when a local
/// stdio server omits its own working directory.
/// environment registry plus the stdio fallback cwd used when a server omits
/// its own working directory.
#[derive(Clone)]
pub struct McpRuntimeContext {
environment_manager: Arc<EnvironmentManager>,
local_stdio_fallback_cwd: PathBuf,
stdio_fallback_cwd: PathBuf,
}
impl McpRuntimeContext {
pub fn new(
environment_manager: Arc<EnvironmentManager>,
local_stdio_fallback_cwd: PathBuf,
) -> Self {
pub fn new(environment_manager: Arc<EnvironmentManager>, stdio_fallback_cwd: PathBuf) -> Self {
Self {
environment_manager,
local_stdio_fallback_cwd,
stdio_fallback_cwd,
}
}
pub(crate) fn local_stdio_fallback_cwd(&self) -> PathBuf {
self.local_stdio_fallback_cwd.clone()
pub(crate) fn stdio_fallback_cwd(&self) -> PathBuf {
self.stdio_fallback_cwd.clone()
}
pub(crate) fn resolve_server_environment(
@@ -67,9 +64,6 @@ impl McpRuntimeContext {
.environment_manager
.get_environment(&config.environment_id)
{
if !config.is_local_environment() {
ensure_remote_stdio_cwd(server_name, config)?;
}
return Ok(Some(environment));
}
@@ -89,27 +83,6 @@ impl McpRuntimeContext {
}
}
fn ensure_remote_stdio_cwd(
server_name: &str,
config: &codex_config::McpServerConfig,
) -> Result<(), String> {
let codex_config::McpServerTransportConfig::Stdio { cwd, .. } = &config.transport else {
return Ok(());
};
let Some(cwd) = cwd else {
return Err(format!(
"remote stdio MCP server `{server_name}` requires an absolute cwd"
));
};
if cwd.is_absolute() {
return Ok(());
}
Err(format!(
"remote stdio MCP server `{server_name}` requires an absolute cwd, got `{}`",
cwd.display()
))
}
pub(crate) fn emit_duration(metric: &str, duration: Duration, tags: &[(&str, &str)]) {
if let Some(metrics) = codex_otel::global() {
let _ = metrics.record_duration(metric, duration, tags);
@@ -233,13 +206,8 @@ mod tests {
PathBuf::from("/tmp"),
);
let mut remote_stdio = stdio_server("remote");
let McpServerTransportConfig::Stdio { cwd, .. } = &mut remote_stdio.transport else {
unreachable!("stdio helper should build stdio transport");
};
*cwd = Some(std::env::temp_dir());
for resolved_runtime in [
runtime_context.resolve_server_environment("stdio", &remote_stdio),
runtime_context.resolve_server_environment("stdio", &stdio_server("remote")),
runtime_context.resolve_server_environment("http", &http_server("remote")),
] {
let resolved_runtime = match resolved_runtime {
@@ -265,32 +233,4 @@ mod tests {
};
assert!(resolved_runtime.is_some());
}
#[tokio::test]
async fn remote_stdio_requires_absolute_cwd() {
let runtime_context = McpRuntimeContext::new(
Arc::new(
EnvironmentManager::create_for_tests(
Some("ws://127.0.0.1:8765".to_string()),
/*local_runtime_paths*/ None,
)
.await,
),
PathBuf::from("/tmp"),
);
let mut remote_stdio = stdio_server("remote");
let McpServerTransportConfig::Stdio { cwd, .. } = &mut remote_stdio.transport else {
unreachable!("stdio helper should build stdio transport");
};
*cwd = Some(PathBuf::from("relative"));
let error = match runtime_context.resolve_server_environment("stdio", &remote_stdio) {
Ok(_) => panic!("remote stdio MCP should require absolute cwd"),
Err(error) => error,
};
assert_eq!(
error,
"remote stdio MCP server `stdio` requires an absolute cwd, got `relative`"
);
}
}

25
codex-rs/config/src/mcp_types.rs
View File

@@ -358,7 +358,6 @@ impl TryFrom<RawMcpServerConfig> for McpServerConfig {
let environment_id =
environment_id.unwrap_or_else(|| DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string());
validate_remote_stdio_cwd(&transport, &environment_id)?;
Ok(Self {
transport,
@@ -395,30 +394,6 @@ const fn default_enabled() -> bool {
true
}
fn validate_remote_stdio_cwd(
transport: &McpServerTransportConfig,
environment_id: &str,
) -> Result<(), String> {
if environment_id == DEFAULT_MCP_SERVER_ENVIRONMENT_ID {
return Ok(());
}
let McpServerTransportConfig::Stdio { cwd, .. } = transport else {
return Ok(());
};
let Some(cwd) = cwd else {
return Err(format!(
"remote stdio MCP servers require an absolute cwd when environment_id is `{environment_id}`"
));
};
if cwd.is_absolute() {
return Ok(());
}
Err(format!(
"remote stdio MCP servers require an absolute cwd when environment_id is `{environment_id}`, got `{}`",
cwd.display()
))
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema)]
#[serde(untagged, deny_unknown_fields, rename_all = "snake_case")]
pub enum McpServerTransportConfig {

56
codex-rs/config/src/mcp_types_tests.rs
View File

@@ -51,62 +51,6 @@ fn deserialize_stdio_command_server_config_with_args() {
assert!(cfg.enabled);
}
#[test]
fn deserialize_remote_stdio_server_requires_absolute_cwd() {
let missing_cwd = toml::from_str::<McpServerConfig>(
r#"
command = "echo"
environment_id = "remote"
"#,
)
.expect_err("remote stdio MCP should require cwd");
assert!(
missing_cwd
.to_string()
.contains("remote stdio MCP servers require an absolute cwd"),
"unexpected error: {missing_cwd}"
);
let relative_cwd = toml::from_str::<McpServerConfig>(
r#"
command = "echo"
environment_id = "remote"
cwd = "relative"
"#,
)
.expect_err("remote stdio MCP should require absolute cwd");
assert!(
relative_cwd.to_string().contains("got `relative`"),
"unexpected error: {relative_cwd}"
);
}
#[test]
fn deserialize_remote_stdio_server_accepts_absolute_cwd() {
let cwd = std::env::temp_dir();
let cfg: McpServerConfig = match toml::from_str(&format!(
r#"
command = "echo"
environment_id = "remote"
cwd = {cwd:?}
"#
)) {
Ok(cfg) => cfg,
Err(error) => panic!("remote stdio MCP should accept absolute cwd: {error}"),
};
assert_eq!(
cfg.transport,
McpServerTransportConfig::Stdio {
command: "echo".to_string(),
args: vec![],
env: None,
env_vars: Vec::new(),
cwd: Some(cwd),
}
);
}
#[test]
fn deserialize_stdio_command_server_config_with_arg_with_args_and_env() {
let cfg: McpServerConfig = toml::from_str(