Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions codex-rs/app-server/src/codex_message_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5802,8 +5802,8 @@ impl CodexMessageProcessor {
let environment_manager = self.thread_manager.environment_manager();
let runtime_environment = match environment_manager.default_environment() {
Some(environment) => {
// Status listing has no turn cwd. This fallback is used only
// by executor-backed stdio MCPs whose config omits `cwd`.
// Status listing has no turn cwd. This fallback is used by
// stdio MCPs whose config omits `cwd`.
McpRuntimeEnvironment::new(environment, config.cwd.to_path_buf())
}
None => McpRuntimeEnvironment::new(
Expand Down
8 changes: 5 additions & 3 deletions codex-rs/codex-mcp/src/mcp_connection_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -666,8 +666,8 @@ pub struct McpConnectionManager {
/// servers should run for the current caller. Keep it explicit at manager
/// construction time so status/snapshot paths and real sessions make the same
/// local-vs-remote decision. `fallback_cwd` is not a per-server override; it is
/// used only when an executor-backed stdio server omits `cwd` and the executor
/// API still needs a concrete process working directory.
/// used when a stdio server omits `cwd` and the launcher needs a concrete
/// process working directory.
#[derive(Clone)]
pub struct McpRuntimeEnvironment {
environment: Arc<Environment>,
Expand Down Expand Up @@ -1573,7 +1573,9 @@ async fn make_rmcp_client(
runtime_environment.fallback_cwd(),
))
} else {
Arc::new(LocalStdioServerLauncher) as Arc<dyn StdioServerLauncher>
Arc::new(LocalStdioServerLauncher::new(
runtime_environment.fallback_cwd(),
)) as Arc<dyn StdioServerLauncher>
};

// `RmcpClient` always sees a launched MCP stdio server. The
Expand Down
63 changes: 63 additions & 0 deletions codex-rs/core/tests/suite/rmcp_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,69 @@ async fn remote_stdio_server_uses_runtime_fallback_cwd_when_config_omits_cwd() -
Ok(())
}

#[cfg(unix)]
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
#[serial(mcp_cwd)]
async fn local_stdio_server_uses_runtime_fallback_cwd_when_config_omits_cwd() -> anyhow::Result<()>
{
skip_if_no_network!(Ok(()));

let server = responses::start_mock_server().await;
let server_name = "rmcp_local_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 = cargo_bin("test_stdio_server")?;
let relative_server_path = PathBuf::from("mcp-bin").join(
rmcp_test_server_bin
.file_name()
.expect("test stdio server binary should have a file name"),
);
let relative_command = relative_server_path.to_string_lossy().into_owned();

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());

let target_bin = config.cwd.join(&relative_server_path).into_path_buf();
let target_dir = target_bin
.parent()
.expect("relative test server path should include a parent");
fs::create_dir_all(target_dir).expect("create relative MCP bin directory");
fs::copy(&rmcp_test_server_bin, &target_bin).expect("copy test stdio server");

insert_mcp_server(
config,
server_name,
stdio_transport(
relative_command,
Some(HashMap::from([(
"MCP_TEST_VALUE".to_string(),
"local-fallback-cwd".to_string(),
)])),
Vec::new(),
),
TestMcpServerOptions::default(),
);
})
.build(&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-local-fallback-cwd").await?;

assert_cwd_tool_output(&structured, &expected_cwd);
server.verify().await;
Ok(())
}

#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
async fn stdio_mcp_tool_call_includes_sandbox_state_meta() -> anyhow::Result<()> {
skip_if_no_network!(Ok(()));
Expand Down
21 changes: 13 additions & 8 deletions codex-rs/rmcp-client/src/program_resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,19 @@

use std::collections::HashMap;
use std::ffi::OsString;
use std::path::Path;

/// Resolves a program to its executable path on Unix systems.
///
/// Unix systems handle PATH resolution and script execution natively through
/// the kernel's shebang (`#!`) mechanism, so this function simply returns
/// the program name unchanged.
#[cfg(unix)]
pub fn resolve(program: OsString, _env: &HashMap<OsString, OsString>) -> std::io::Result<OsString> {
pub fn resolve(
program: OsString,
_env: &HashMap<OsString, OsString>,
_cwd: &Path,
) -> std::io::Result<OsString> {
Ok(program)
}

Expand All @@ -33,16 +38,16 @@ pub fn resolve(program: OsString, _env: &HashMap<OsString, OsString>) -> std::io
/// This enables tools like `npx`, `pnpm`, and `yarn` to work correctly on Windows
/// without requiring users to specify full paths or extensions in their configuration.
#[cfg(windows)]
pub fn resolve(program: OsString, env: &HashMap<OsString, OsString>) -> std::io::Result<OsString> {
// Get current directory for relative path resolution
let cwd = std::env::current_dir()
.map_err(|e| std::io::Error::other(format!("Failed to get current directory: {e}")))?;

pub fn resolve(
program: OsString,
env: &HashMap<OsString, OsString>,
cwd: &Path,
) -> std::io::Result<OsString> {
// Extract PATH from environment for search locations
let search_path = env.get(std::ffi::OsStr::new("PATH"));

// Attempt resolution via which crate
match which::which_in(&program, search_path, &cwd) {
match which::which_in(&program, search_path, cwd) {
Ok(resolved) => {
tracing::debug!("Resolved {program:?} to {resolved:?}");
Ok(resolved.into_os_string())
Expand Down Expand Up @@ -119,7 +124,7 @@ mod tests {
let program = OsString::from(&env.program_name);

// Apply platform-specific resolution
let resolved = resolve(program, &env.mcp_env)?;
let resolved = resolve(program, &env.mcp_env, std::env::current_dir()?.as_path())?;

// Verify resolved path executes successfully
let mut cmd = Command::new(resolved);
Expand Down
29 changes: 22 additions & 7 deletions codex-rs/rmcp-client/src/stdio_server_launcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,14 +162,27 @@ impl StdioServerCommand {
/// process spawns the configured command and rmcp talks to the child's local
/// stdin/stdout pipes directly.
#[derive(Clone)]
pub struct LocalStdioServerLauncher;
pub struct LocalStdioServerLauncher {
fallback_cwd: PathBuf,
}

impl LocalStdioServerLauncher {
/// Creates a local stdio launcher.
///
/// `fallback_cwd` is used when the MCP server config omits `cwd`, so
/// relative commands resolve from the caller's runtime working directory.
pub fn new(fallback_cwd: PathBuf) -> Self {
Self { fallback_cwd }
}
}

impl StdioServerLauncher for LocalStdioServerLauncher {
fn launch(
&self,
command: StdioServerCommand,
) -> BoxFuture<'static, io::Result<StdioServerTransport>> {
async move { Self::launch_server(command) }.boxed()
let fallback_cwd = self.fallback_cwd.clone();
async move { Self::launch_server(command, fallback_cwd) }.boxed()
}
}

Expand All @@ -193,7 +206,10 @@ mod private {
impl private::Sealed for LocalStdioServerLauncher {}

impl LocalStdioServerLauncher {
fn launch_server(command: StdioServerCommand) -> io::Result<StdioServerTransport> {
fn launch_server(
command: StdioServerCommand,
fallback_cwd: PathBuf,
) -> io::Result<StdioServerTransport> {
let StdioServerCommand {
program,
args,
Expand All @@ -203,22 +219,21 @@ impl LocalStdioServerLauncher {
} = command;
let program_name = program.to_string_lossy().into_owned();
let envs = create_env_for_mcp_server(env, &env_vars).map_err(io::Error::other)?;
let cwd = cwd.unwrap_or(fallback_cwd);
let resolved_program =
program_resolver::resolve(program, &envs).map_err(io::Error::other)?;
program_resolver::resolve(program, &envs, &cwd).map_err(io::Error::other)?;

let mut command = Command::new(resolved_program);
command
.kill_on_drop(true)
.stdin(Stdio::piped())
.stdout(Stdio::piped())
.current_dir(cwd)
.env_clear()
.envs(envs)
.args(args);
#[cfg(unix)]
command.process_group(0);
if let Some(cwd) = cwd {
command.current_dir(cwd);
}

let (transport, stderr) = TokioChildProcess::builder(command)
.stderr(Stdio::piped())
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/rmcp-client/tests/process_group_cleanup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ async fn drop_kills_wrapper_process_group() -> Result<()> {
)])),
&[],
/*cwd*/ None,
Arc::new(LocalStdioServerLauncher),
Arc::new(LocalStdioServerLauncher::new(std::env::current_dir()?)),
)
.await?;

Expand Down
2 changes: 1 addition & 1 deletion codex-rs/rmcp-client/tests/resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ async fn rmcp_client_can_list_and_read_resources() -> anyhow::Result<()> {
/*env*/ None,
&[],
/*cwd*/ None,
Arc::new(LocalStdioServerLauncher),
Arc::new(LocalStdioServerLauncher::new(std::env::current_dir()?)),
)
.await?;

Expand Down
Loading