fix: keep rmcp-client env vars as OsString

codex-rs/core/src/mcp_connection_manager.rs

@@ -1440,7 +1440,12 @@ async fn make_rmcp_client(
         } => {
             let command_os: OsString = command.into();
             let args_os: Vec<OsString> = args.into_iter().map(Into::into).collect();
-            RmcpClient::new_stdio_client(command_os, args_os, env, &env_vars, cwd)
+            let env_os = env.map(|env| {
+                env.into_iter()
+                    .map(|(key, value)| (key.into(), value.into()))
+                    .collect::<HashMap<_, _>>()
+            });
+            RmcpClient::new_stdio_client(command_os, args_os, env_os, &env_vars, cwd)
                 .await
                 .map_err(|err| StartupOutcomeError::from(anyhow!(err)))
         }

codex-rs/rmcp-client/src/program_resolver.rs

@@ -13,18 +13,13 @@
 use std::collections::HashMap;
 use std::ffi::OsString;
 
-#[cfg(windows)]
-use std::env;
-#[cfg(windows)]
-use tracing::debug;
-
 /// 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<String, String>) -> std::io::Result<OsString> {
+pub fn resolve(program: OsString, _env: &HashMap<OsString, OsString>) -> std::io::Result<OsString> {
     Ok(program)
 }
 
@@ -38,25 +33,22 @@ pub fn resolve(program: OsString, _env: &HashMap<String, String>) -> std::io::Re
 /// 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<String, String>) -> std::io::Result<OsString> {
+pub fn resolve(program: OsString, env: &HashMap<OsString, OsString>) -> std::io::Result<OsString> {
     // Get current directory for relative path resolution
-    let cwd = env::current_dir()
+    let cwd = std::env::current_dir()
         .map_err(|e| std::io::Error::other(format!("Failed to get current directory: {e}")))?;
 
     // Extract PATH from environment for search locations
-    let search_path = env.get("PATH");
+    let search_path = env.get(std::ffi::OsStr::new("PATH"));
 
     // Attempt resolution via which crate
     match which::which_in(&program, search_path, &cwd) {
         Ok(resolved) => {
-            debug!("Resolved {:?} to {:?}", program, resolved);
+            tracing::debug!("Resolved {program:?} to {resolved:?}");
             Ok(resolved.into_os_string())
         }
         Err(e) => {
-            debug!(
-                "Failed to resolve {:?}: {}. Using original path",
-                program, e
-            );
+            tracing::debug!("Failed to resolve {program:?}: {e}. Using original path");
             // Fallback to original program - let Command::new() handle the error
             Ok(program)
         }
@@ -146,7 +138,7 @@ mod tests {
         // Held to prevent the temporary directory from being deleted.
         _temp_dir: TempDir,
         program_name: String,
-        mcp_env: HashMap<String, String>,
+        mcp_env: HashMap<OsString, OsString>,
     }
 
     impl TestExecutableEnv {
@@ -160,10 +152,10 @@ mod tests {
 
             // Build a clean environment with the temp dir in the PATH.
             let mut extra_env = HashMap::new();
-            extra_env.insert("PATH".to_string(), Self::build_path(dir_path));
+            extra_env.insert(OsString::from("PATH"), Self::build_path_env_var(dir_path));
 
             #[cfg(windows)]
-            extra_env.insert("PATHEXT".to_string(), Self::ensure_cmd_extension());
+            extra_env.insert(OsString::from("PATHEXT"), Self::ensure_cmd_extension());
 
             let mcp_env = create_env_for_mcp_server(Some(extra_env), &[]);
 
@@ -202,20 +194,30 @@ mod tests {
         }
 
         /// Prepends the given directory to the system's PATH variable.
-        fn build_path(dir: &Path) -> String {
-            let current = std::env::var("PATH").unwrap_or_default();
-            let sep = if cfg!(windows) { ";" } else { ":" };
-            format!("{}{sep}{current}", dir.to_string_lossy())
+        fn build_path_env_var(dir: &Path) -> OsString {
+            let mut path = OsString::from(dir.as_os_str());
+            if let Some(current) = std::env::var_os("PATH") {
+                let sep = if cfg!(windows) { ";" } else { ":" };
+                path.push(sep);
+                path.push(current);
+            }
+            path
         }
 
         /// Ensures `.CMD` is in the `PATHEXT` variable on Windows for script discovery.
         #[cfg(windows)]
-        fn ensure_cmd_extension() -> String {
-            let current = std::env::var("PATHEXT").unwrap_or_default();
-            if current.to_uppercase().contains(".CMD") {
+        fn ensure_cmd_extension() -> OsString {
+            let current = std::env::var_os("PATHEXT").unwrap_or_default();
+            if current
+                .to_string_lossy()
+                .to_ascii_uppercase()
+                .contains(".CMD")
+            {
                 current
             } else {
-                format!(".CMD;{current}")
+                let mut path_ext = OsString::from(".CMD;");
+                path_ext.push(current);
+                path_ext
             }
         }
     }

codex-rs/rmcp-client/src/rmcp_client.rs

@@ -390,7 +390,7 @@ enum TransportRecipe {
     Stdio {
         program: OsString,
         args: Vec<OsString>,
-        env: Option<HashMap<String, String>>,
+        env: Option<HashMap<OsString, OsString>>,
         env_vars: Vec<String>,
         cwd: Option<PathBuf>,
     },
@@ -478,7 +478,7 @@ impl RmcpClient {
     pub async fn new_stdio_client(
         program: OsString,
         args: Vec<OsString>,
-        env: Option<HashMap<String, String>>,
+        env: Option<HashMap<OsString, OsString>>,
         env_vars: &[String],
         cwd: Option<PathBuf>,
     ) -> io::Result<Self> {

codex-rs/rmcp-client/src/utils.rs

@@ -5,16 +5,17 @@ use reqwest::header::HeaderName;
 use reqwest::header::HeaderValue;
 use std::collections::HashMap;
 use std::env;
+use std::ffi::OsString;
 
 pub(crate) fn create_env_for_mcp_server(
-    extra_env: Option<HashMap<String, String>>,
+    extra_env: Option<HashMap<OsString, OsString>>,
     env_vars: &[String],
-) -> HashMap<String, String> {
+) -> HashMap<OsString, OsString> {
     DEFAULT_ENV_VARS
         .iter()
         .copied()
         .chain(env_vars.iter().map(String::as_str))
-        .filter_map(|var| env::var(var).ok().map(|value| (var.to_string(), value)))
+        .filter_map(|var| env::var_os(var).map(|value| (OsString::from(var), value)))
         .chain(extra_env.unwrap_or_default())
         .collect()
 }
@@ -140,18 +141,18 @@ mod tests {
     use pretty_assertions::assert_eq;
 
     use serial_test::serial;
-    use std::ffi::OsString;
+    use std::ffi::OsStr;
 
     struct EnvVarGuard {
         key: String,
         original: Option<OsString>,
     }
 
     impl EnvVarGuard {
-        fn set(key: &str, value: &str) -> Self {
+        fn set(key: &str, value: impl AsRef<OsStr>) -> Self {
             let original = std::env::var_os(key);
             unsafe {
-                std::env::set_var(key, value);
+                std::env::set_var(key, value.as_ref());
             }
             Self {
                 key: key.to_string(),
@@ -177,18 +178,37 @@ mod tests {
     #[tokio::test]
     async fn create_env_honors_overrides() {
         let value = "custom".to_string();
-        let env =
-            create_env_for_mcp_server(Some(HashMap::from([("TZ".into(), value.clone())])), &[]);
-        assert_eq!(env.get("TZ"), Some(&value));
+        let expected = OsString::from(&value);
+        let env = create_env_for_mcp_server(
+            Some(HashMap::from([(OsString::from("TZ"), expected.clone())])),
+            &[],
+        );
+        assert_eq!(env.get(OsStr::new("TZ")), Some(&expected));
     }
 
     #[test]
     #[serial(extra_rmcp_env)]
     fn create_env_includes_additional_whitelisted_variables() {
         let custom_var = "EXTRA_RMCP_ENV";
         let value = "from-env";
+        let expected = OsString::from(value);
         let _guard = EnvVarGuard::set(custom_var, value);
         let env = create_env_for_mcp_server(None, &[custom_var.to_string()]);
-        assert_eq!(env.get(custom_var), Some(&value.to_string()));
+        assert_eq!(env.get(OsStr::new(custom_var)), Some(&expected));
+    }
+
+    #[cfg(unix)]
+    #[test]
+    #[serial(extra_rmcp_env)]
+    fn create_env_preserves_path_when_it_is_not_utf8() {
+        use std::os::unix::ffi::OsStrExt;
+
+        let raw_path = std::ffi::OsStr::from_bytes(b"/tmp/codex-\xFF/bin");
+        let expected = raw_path.to_os_string();
+        let _guard = EnvVarGuard::set("PATH", raw_path);
+
+        let env = create_env_for_mcp_server(None, &[]);
+
+        assert_eq!(env.get(OsStr::new("PATH")), Some(&expected));
     }
 }

codex-rs/rmcp-client/tests/process_group_cleanup.rs

@@ -73,8 +73,8 @@ async fn drop_kills_wrapper_process_group() -> Result<()> {
             ),
         ],
         Some(HashMap::from([(
-            "CHILD_PID_FILE".to_string(),
-            child_pid_file_str,
+            OsString::from("CHILD_PID_FILE"),
+            OsString::from(child_pid_file_str),
         )])),
         &[],
         None,