mirror of
https://github.com/openai/codex.git
synced 2026-08-23 13:09:46 +00:00
mcp: classify stdio cwd by launcher
This commit is contained in:
@@ -55,9 +55,11 @@ use codex_protocol::protocol::McpStartupUpdateEvent;
|
||||
use codex_rmcp_client::ExecutorStdioServerLauncher;
|
||||
use codex_rmcp_client::LocalStdioServerLauncher;
|
||||
use codex_rmcp_client::RmcpClient;
|
||||
use codex_rmcp_client::StdioServerCwd;
|
||||
use codex_rmcp_client::StdioServerLauncher;
|
||||
use codex_rmcp_client::ToolWithConnectorId;
|
||||
use codex_rmcp_client::is_authentication_required_error;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use futures::future::BoxFuture;
|
||||
use futures::future::FutureExt;
|
||||
use futures::future::Shared;
|
||||
@@ -970,25 +972,54 @@ async fn make_rmcp_client(
|
||||
.map(|(key, value)| (key.into(), value.into()))
|
||||
.collect::<HashMap<_, _>>()
|
||||
});
|
||||
let launcher = if is_local_environment {
|
||||
let (cwd, launcher) = if is_local_environment {
|
||||
// TODO(starr): Unify local stdio MCP launch with
|
||||
// `ExecutorStdioServerLauncher` once the executor-backed path
|
||||
// preserves `LocalStdioServerLauncher` semantics.
|
||||
Arc::new(LocalStdioServerLauncher::new(
|
||||
runtime_context.local_stdio_fallback_cwd(),
|
||||
)) as Arc<dyn StdioServerLauncher>
|
||||
let cwd = cwd
|
||||
.map(|cwd| {
|
||||
let configured_cwd = cwd.as_str().to_string();
|
||||
cwd.to_host_abs_path()
|
||||
.map(|cwd| StdioServerCwd::Local(cwd.into_path_buf()))
|
||||
.map_err(|err| {
|
||||
StartupOutcomeError::from(anyhow!(
|
||||
"invalid cwd `{configured_cwd}` for local stdio MCP server `{server_name}`: {err}"
|
||||
))
|
||||
})
|
||||
})
|
||||
.transpose()?;
|
||||
(
|
||||
cwd,
|
||||
Arc::new(LocalStdioServerLauncher::new(
|
||||
runtime_context.local_stdio_fallback_cwd(),
|
||||
)) as Arc<dyn StdioServerLauncher>,
|
||||
)
|
||||
} else {
|
||||
let Some(environment) = resolved_environment.as_ref() else {
|
||||
unreachable!(
|
||||
"non-local stdio MCP servers resolve an environment before launch"
|
||||
);
|
||||
};
|
||||
Arc::new(ExecutorStdioServerLauncher::new(
|
||||
environment.get_exec_backend(),
|
||||
)) as Arc<dyn StdioServerLauncher>
|
||||
let cwd = cwd
|
||||
.map(|cwd| {
|
||||
let configured_cwd = cwd.as_str().to_string();
|
||||
PathUri::try_from(cwd)
|
||||
.map(StdioServerCwd::Executor)
|
||||
.map_err(|err| {
|
||||
StartupOutcomeError::from(anyhow!(
|
||||
"invalid cwd `{configured_cwd}` for executor stdio MCP server `{server_name}`: {err}"
|
||||
))
|
||||
})
|
||||
})
|
||||
.transpose()?;
|
||||
(
|
||||
cwd,
|
||||
Arc::new(ExecutorStdioServerLauncher::new(
|
||||
environment.get_exec_backend(),
|
||||
)) as Arc<dyn StdioServerLauncher>,
|
||||
)
|
||||
};
|
||||
|
||||
let cwd = cwd.map(codex_utils_path_uri::LegacyAppPathString::into_string);
|
||||
RmcpClient::new_stdio_client(command_os, args_os, env_os, &env_vars, cwd, launcher)
|
||||
.await
|
||||
.map_err(|err| StartupOutcomeError::from(anyhow!(err)))
|
||||
|
||||
@@ -44,4 +44,5 @@ pub use rmcp_client::ToolWithConnectorId;
|
||||
pub use startup_error::is_authentication_required_error;
|
||||
pub use stdio_server_launcher::ExecutorStdioServerLauncher;
|
||||
pub use stdio_server_launcher::LocalStdioServerLauncher;
|
||||
pub use stdio_server_launcher::StdioServerCwd;
|
||||
pub use stdio_server_launcher::StdioServerLauncher;
|
||||
|
||||
@@ -69,6 +69,7 @@ use crate::oauth::OAuthPersistor;
|
||||
use crate::oauth::StoredOAuthTokens;
|
||||
use crate::oauth_http_client::OAuthHttpClientAdapter;
|
||||
use crate::stdio_server_launcher::StdioServerCommand;
|
||||
use crate::stdio_server_launcher::StdioServerCwd;
|
||||
use crate::stdio_server_launcher::StdioServerLauncher;
|
||||
use crate::stdio_server_launcher::StdioServerProcessHandle;
|
||||
use crate::stdio_server_launcher::StdioServerTransport;
|
||||
@@ -353,7 +354,7 @@ impl RmcpClient {
|
||||
args: Vec<OsString>,
|
||||
env: Option<HashMap<OsString, OsString>>,
|
||||
env_vars: &[McpServerEnvVar],
|
||||
cwd: Option<String>,
|
||||
cwd: Option<StdioServerCwd>,
|
||||
launcher: Arc<dyn StdioServerLauncher>,
|
||||
) -> io::Result<Self> {
|
||||
let transport_recipe = TransportRecipe::Stdio {
|
||||
|
||||
@@ -15,7 +15,6 @@ use std::collections::HashMap;
|
||||
use std::ffi::OsString;
|
||||
use std::future::Future;
|
||||
use std::io;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::process::Stdio;
|
||||
use std::sync::Arc;
|
||||
@@ -36,7 +35,6 @@ use codex_exec_server::ExecEnvPolicy;
|
||||
use codex_exec_server::ExecParams;
|
||||
use codex_exec_server::ExecProcess;
|
||||
use codex_protocol::config_types::ShellEnvironmentPolicyInherit;
|
||||
use codex_utils_path_uri::LegacyAppPathString;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
#[cfg(unix)]
|
||||
use codex_utils_pty::process_group::kill_process_group;
|
||||
@@ -84,7 +82,16 @@ pub struct StdioServerCommand {
|
||||
args: Vec<OsString>,
|
||||
env: Option<HashMap<OsString, OsString>>,
|
||||
env_vars: Vec<McpServerEnvVar>,
|
||||
cwd: Option<String>,
|
||||
cwd: Option<StdioServerCwd>,
|
||||
}
|
||||
|
||||
/// Working directory for an MCP stdio server, classified by process placement.
|
||||
#[derive(Clone)]
|
||||
pub enum StdioServerCwd {
|
||||
/// Host-local path spelling. Relative paths are resolved by the local launcher.
|
||||
Local(PathBuf),
|
||||
/// Canonical working directory owned by an executor environment.
|
||||
Executor(PathUri),
|
||||
}
|
||||
|
||||
/// Client-side rmcp transport for a launched MCP stdio server.
|
||||
@@ -151,7 +158,7 @@ impl StdioServerCommand {
|
||||
args: Vec<OsString>,
|
||||
env: Option<HashMap<OsString, OsString>>,
|
||||
env_vars: Vec<McpServerEnvVar>,
|
||||
cwd: Option<String>,
|
||||
cwd: Option<StdioServerCwd>,
|
||||
) -> Self {
|
||||
Self {
|
||||
program,
|
||||
@@ -249,7 +256,16 @@ 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.map(PathBuf::from).unwrap_or(fallback_cwd);
|
||||
let cwd = match cwd {
|
||||
Some(StdioServerCwd::Local(cwd)) => cwd,
|
||||
Some(StdioServerCwd::Executor(_)) => {
|
||||
return Err(io::Error::new(
|
||||
io::ErrorKind::InvalidInput,
|
||||
"local stdio server requires a local cwd",
|
||||
));
|
||||
}
|
||||
None => fallback_cwd,
|
||||
};
|
||||
let resolved_program =
|
||||
program_resolver::resolve(program, &envs, &cwd).map_err(io::Error::other)?;
|
||||
|
||||
@@ -476,14 +492,21 @@ impl ExecutorStdioServerLauncher {
|
||||
env_vars,
|
||||
cwd,
|
||||
} = command;
|
||||
let Some(cwd) = cwd else {
|
||||
return Err(io::Error::other(
|
||||
"executor stdio server requires an explicit cwd",
|
||||
));
|
||||
let cwd = match cwd {
|
||||
Some(StdioServerCwd::Executor(cwd)) => cwd,
|
||||
Some(StdioServerCwd::Local(_)) => {
|
||||
return Err(io::Error::new(
|
||||
io::ErrorKind::InvalidInput,
|
||||
"executor stdio server requires an executor cwd",
|
||||
));
|
||||
}
|
||||
None => {
|
||||
return Err(io::Error::new(
|
||||
io::ErrorKind::InvalidInput,
|
||||
"executor stdio server requires an explicit cwd",
|
||||
));
|
||||
}
|
||||
};
|
||||
let cwd: PathUri = LegacyAppPathString::from_path(Path::new(&cwd))
|
||||
.try_into()
|
||||
.map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err))?;
|
||||
let program_name = program.to_string_lossy().into_owned();
|
||||
let envs = create_env_overlay_for_remote_mcp_server(env, &env_vars);
|
||||
let remote_env_vars = remote_mcp_env_var_names(&env_vars);
|
||||
|
||||
@@ -8,6 +8,7 @@ use codex_exec_server::ExecParams;
|
||||
use codex_exec_server::ExecServerError;
|
||||
use codex_rmcp_client::ExecutorStdioServerLauncher;
|
||||
use codex_rmcp_client::RmcpClient;
|
||||
use codex_rmcp_client::StdioServerCwd;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
@@ -33,10 +34,6 @@ impl ExecBackend for RecordingExecBackend {
|
||||
|
||||
#[tokio::test]
|
||||
async fn executor_stdio_forwards_foreign_absolute_cwd_as_path_uri() {
|
||||
#[cfg(not(windows))]
|
||||
let cwd = r"C:\Users\openai\share";
|
||||
#[cfg(windows)]
|
||||
let cwd = "/home/openai/share";
|
||||
#[cfg(not(windows))]
|
||||
let expected_cwd: PathUri = "file:///C:/Users/openai/share"
|
||||
.parse()
|
||||
@@ -53,7 +50,7 @@ async fn executor_stdio_forwards_foreign_absolute_cwd_as_path_uri() {
|
||||
Vec::new(),
|
||||
/*env*/ None,
|
||||
&[],
|
||||
Some(cwd.to_string()),
|
||||
Some(StdioServerCwd::Executor(expected_cwd.clone())),
|
||||
launcher,
|
||||
)
|
||||
.await;
|
||||
|
||||
Reference in New Issue
Block a user