fix: keep rmcp-client env vars as OsString

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

@@ -16,6 +16,8 @@ use std::ffi::OsString;
 #[cfg(windows)]
 use std::env;
 #[cfg(windows)]
+use std::ffi::OsStr;
+#[cfg(windows)]
 use tracing::debug;
 
 /// Resolves a program to its executable path on Unix systems.
@@ -24,7 +26,7 @@ use tracing::debug;
 /// 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,13 +40,13 @@ 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()
         .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(OsStr::new("PATH"));
 
     // Attempt resolution via which crate
     match which::which_in(&program, search_path, &cwd) {
@@ -146,7 +148,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 +162,13 @@ 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().into(),
+            );
 
             let mcp_env = create_env_for_mcp_server(Some(extra_env), &[]);
 
@@ -202,10 +207,14 @@ 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.

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

@@ -863,7 +863,14 @@ impl RmcpClient {
                 cwd,
             } => {
                 let program_name = program.to_string_lossy().into_owned();
-                let envs = create_env_for_mcp_server(env.clone(), env_vars);
+                let envs = create_env_for_mcp_server(
+                    env.clone().map(|env| {
+                        env.into_iter()
+                            .map(|(key, value)| (key.into(), value.into()))
+                            .collect::<HashMap<_, _>>()
+                    }),
+                    env_vars,
+                );
                 let resolved_program = program_resolver::resolve(program.clone(), &envs)?;
 
                 let mut command = Command::new(resolved_program);

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,7 +141,7 @@ mod tests {
     use pretty_assertions::assert_eq;
 
     use serial_test::serial;
-    use std::ffi::OsString;
+    use std::ffi::OsStr;
 
     struct EnvVarGuard {
         key: String,
@@ -158,6 +159,18 @@ mod tests {
                 original,
             }
         }
+
+        #[cfg(unix)]
+        fn set_os(key: &str, value: &std::ffi::OsStr) -> Self {
+            let original = std::env::var_os(key);
+            unsafe {
+                std::env::set_var(key, value);
+            }
+            Self {
+                key: key.to_string(),
+                original,
+            }
+        }
     }
 
     impl Drop for EnvVarGuard {
@@ -177,9 +190,12 @@ 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]
@@ -187,8 +203,24 @@ mod tests {
     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_os("PATH", raw_path);
+
+        let env = create_env_for_mcp_server(None, &[]);
+
+        assert_eq!(env.get(OsStr::new("PATH")), Some(&expected));
     }
 }