mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
mcp: accept foreign absolute cwd for remote stdio (#29493)
## Why Remote stdio MCP servers can run in an environment whose path convention differs from the Codex host. A Windows cwd such as `C:\Users\openai\share` is absolute for the executor but was rejected by a POSIX orchestrator. Built on #29501, now merged, which only clarifies the host-native `PathUri` constructor name. ## What changed - Deserialize MCP cwd values as `LegacyAppPathString` so config does not apply host path rules. - Interpret that spelling as host-native for local launches and convert it to `PathUri` at executor launch. - Skip host filesystem and command resolution checks for remote stdio in `codex doctor`. - Add host-independent config and executor-boundary coverage using the foreign path convention for each test platform. ## Validation - `just test -p codex-utils-path-uri -p codex-config -p codex-mcp -p codex-rmcp-client` (408 passed) - `just test -p codex-cli -p codex-rmcp-client` (372 passed) - `cargo check --workspace --tests` - `just test` (11,311 passed; 43 unrelated environment/timing failures) - `just fix -p codex-cli -p codex-config -p codex-core -p codex-mcp -p codex-mcp-extension -p codex-rmcp-client -p codex-tui`
This commit is contained in:
@@ -146,7 +146,7 @@ fn serialize_mcp_server(config: &McpServerConfig) -> TomlItem {
|
||||
entry["env_vars"] = array_from_env_vars(env_vars);
|
||||
}
|
||||
if let Some(cwd) = cwd {
|
||||
entry["cwd"] = value(cwd.to_string_lossy().to_string());
|
||||
entry["cwd"] = value(cwd.as_str());
|
||||
}
|
||||
}
|
||||
McpServerTransportConfig::StreamableHttp {
|
||||
|
||||
@@ -2,9 +2,9 @@
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::fmt;
|
||||
use std::path::PathBuf;
|
||||
use std::time::Duration;
|
||||
|
||||
use codex_utils_path_uri::LegacyAppPathString;
|
||||
use schemars::JsonSchema;
|
||||
use serde::Deserialize;
|
||||
use serde::Deserializer;
|
||||
@@ -224,7 +224,7 @@ pub struct RawMcpServerConfig {
|
||||
#[serde(default)]
|
||||
pub env_vars: Option<Vec<McpServerEnvVar>>,
|
||||
#[serde(default)]
|
||||
pub cwd: Option<PathBuf>,
|
||||
pub cwd: Option<LegacyAppPathString>,
|
||||
pub http_headers: Option<HashMap<String, String>>,
|
||||
#[serde(default)]
|
||||
pub env_http_headers: Option<HashMap<String, String>>,
|
||||
@@ -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 {
|
||||
@@ -432,7 +407,7 @@ pub enum McpServerTransportConfig {
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
env_vars: Vec<McpServerEnvVar>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
cwd: Option<PathBuf>,
|
||||
cwd: Option<LegacyAppPathString>,
|
||||
},
|
||||
/// https://modelcontextprotocol.io/specification/2025-06-18/basic/transports#streamable-http
|
||||
StreamableHttp {
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
use super::*;
|
||||
use codex_utils_path_uri::LegacyAppPathString;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::collections::HashMap;
|
||||
use std::path::PathBuf;
|
||||
use std::path::Path;
|
||||
|
||||
#[test]
|
||||
fn deserialize_stdio_command_server_config() {
|
||||
@@ -52,38 +53,12 @@ fn deserialize_stdio_command_server_config_with_args() {
|
||||
}
|
||||
|
||||
#[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();
|
||||
fn deserialize_remote_stdio_server_accepts_foreign_absolute_cwd() {
|
||||
#[cfg(not(windows))]
|
||||
let cwd = r"C:\Users\openai\share";
|
||||
#[cfg(windows)]
|
||||
let cwd = "/home/openai/share";
|
||||
let expected_cwd = LegacyAppPathString::from_path(Path::new(cwd));
|
||||
let cfg: McpServerConfig = match toml::from_str(&format!(
|
||||
r#"
|
||||
command = "echo"
|
||||
@@ -102,7 +77,7 @@ fn deserialize_remote_stdio_server_accepts_absolute_cwd() {
|
||||
args: vec![],
|
||||
env: None,
|
||||
env_vars: Vec::new(),
|
||||
cwd: Some(cwd),
|
||||
cwd: Some(expected_cwd),
|
||||
}
|
||||
);
|
||||
}
|
||||
@@ -223,7 +198,7 @@ fn deserialize_stdio_command_server_config_with_cwd() {
|
||||
args: vec![],
|
||||
env: None,
|
||||
env_vars: Vec::new(),
|
||||
cwd: Some(PathBuf::from("/tmp")),
|
||||
cwd: Some(LegacyAppPathString::from_path(Path::new("/tmp"))),
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user