Merge branch 'main' into goden/fix_issue_6700

.github/workflows/rust-ci.yml

@@ -95,9 +95,8 @@ jobs:
       run:
         working-directory: codex-rs
     env:
-      # Speed up repeated builds across CI runs by caching compiled objects (non-Windows).
-      USE_SCCACHE: ${{ startsWith(matrix.runner, 'windows') && 'false' || 'true' }}
-      RUSTC_WRAPPER: ${{ startsWith(matrix.runner, 'windows') && '' || 'sccache' }}
+      # Speed up repeated builds across CI runs by caching compiled objects.
+      RUSTC_WRAPPER: sccache
       CARGO_INCREMENTAL: "0"
       SCCACHE_CACHE_SIZE: 10G
 
@@ -171,14 +170,12 @@ jobs:
 
       # Install and restore sccache cache
       - name: Install sccache
-        if: ${{ env.USE_SCCACHE == 'true' }}
         uses: taiki-e/install-action@44c6d64aa62cd779e873306675c7a58e86d6d532 # v2
         with:
           tool: sccache
           version: 0.7.5
 
       - name: Configure sccache backend
-        if: ${{ env.USE_SCCACHE == 'true' }}
         shell: bash
         run: |
           set -euo pipefail
@@ -192,7 +189,7 @@ jobs:
           fi
 
       - name: Restore sccache cache (fallback)
-        if: ${{ env.USE_SCCACHE == 'true' && env.SCCACHE_GHA_ENABLED != 'true' }}
+        if: ${{ env.SCCACHE_GHA_ENABLED != 'true' }}
         id: cache_sccache_restore
         uses: actions/cache/restore@v4
         with:
@@ -277,7 +274,7 @@ jobs:
           key: cargo-home-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}-${{ hashFiles('**/Cargo.lock') }}-${{ hashFiles('codex-rs/rust-toolchain.toml') }}
 
       - name: Save sccache cache (fallback)
-        if: always() && !cancelled() && env.USE_SCCACHE == 'true' && env.SCCACHE_GHA_ENABLED != 'true'
+        if: always() && !cancelled() && env.SCCACHE_GHA_ENABLED != 'true'
         continue-on-error: true
         uses: actions/cache/save@v4
         with:
@@ -285,12 +282,12 @@ jobs:
           key: sccache-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}-${{ hashFiles('**/Cargo.lock') }}-${{ github.run_id }}
 
       - name: sccache stats
-        if: always() && env.USE_SCCACHE == 'true'
+        if: always()
         continue-on-error: true
         run: sccache --show-stats || true
 
       - name: sccache summary
-        if: always() && env.USE_SCCACHE == 'true'
+        if: always()
         shell: bash
         run: |
           {
@@ -329,9 +326,7 @@ jobs:
       run:
         working-directory: codex-rs
     env:
-      # Speed up repeated builds across CI runs by caching compiled objects (non-Windows).
-      USE_SCCACHE: ${{ startsWith(matrix.runner, 'windows') && 'false' || 'true' }}
-      RUSTC_WRAPPER: ${{ startsWith(matrix.runner, 'windows') && '' || 'sccache' }}
+      RUSTC_WRAPPER: sccache
       CARGO_INCREMENTAL: "0"
       SCCACHE_CACHE_SIZE: 10G
 
@@ -375,14 +370,12 @@ jobs:
             cargo-home-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}-
 
       - name: Install sccache
-        if: ${{ env.USE_SCCACHE == 'true' }}
         uses: taiki-e/install-action@44c6d64aa62cd779e873306675c7a58e86d6d532 # v2
         with:
           tool: sccache
           version: 0.7.5
 
       - name: Configure sccache backend
-        if: ${{ env.USE_SCCACHE == 'true' }}
         shell: bash
         run: |
           set -euo pipefail
@@ -396,7 +389,7 @@ jobs:
           fi
 
       - name: Restore sccache cache (fallback)
-        if: ${{ env.USE_SCCACHE == 'true' && env.SCCACHE_GHA_ENABLED != 'true' }}
+        if: ${{ env.SCCACHE_GHA_ENABLED != 'true' }}
         id: cache_sccache_restore
         uses: actions/cache/restore@v4
         with:
@@ -431,7 +424,7 @@ jobs:
           key: cargo-home-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}-${{ hashFiles('**/Cargo.lock') }}-${{ hashFiles('codex-rs/rust-toolchain.toml') }}
 
       - name: Save sccache cache (fallback)
-        if: always() && !cancelled() && env.USE_SCCACHE == 'true' && env.SCCACHE_GHA_ENABLED != 'true'
+        if: always() && !cancelled() && env.SCCACHE_GHA_ENABLED != 'true'
         continue-on-error: true
         uses: actions/cache/save@v4
         with:
@@ -439,12 +432,12 @@ jobs:
           key: sccache-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}-${{ hashFiles('**/Cargo.lock') }}-${{ github.run_id }}
 
       - name: sccache stats
-        if: always() && env.USE_SCCACHE == 'true'
+        if: always()
         continue-on-error: true
         run: sccache --show-stats || true
 
       - name: sccache summary
-        if: always() && env.USE_SCCACHE == 'true'
+        if: always()
         shell: bash
         run: |
           {

codex-rs/Cargo.lock

@@ -1427,6 +1427,7 @@ dependencies = [
  "tracing",
  "urlencoding",
  "webbrowser",
+ "which",
 ]
 
 [[package]]

codex-rs/rmcp-client/Cargo.toml

@@ -56,6 +56,7 @@ tokio = { workspace = true, features = [
 tracing = { workspace = true, features = ["log"] }
 urlencoding = { workspace = true }
 webbrowser = { workspace = true }
+which = { workspace = true }
 
 [dev-dependencies]
 escargot = { workspace = true }

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

@@ -3,6 +3,7 @@ mod find_codex_home;
 mod logging_client_handler;
 mod oauth;
 mod perform_oauth_login;
+mod program_resolver;
 mod rmcp_client;
 mod utils;
 

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

@@ -0,0 +1,222 @@
+//! Platform-specific program resolution for MCP server execution.
+//!
+//! This module provides a unified interface for resolving executable paths
+//! across different operating systems. The key challenge it addresses is that
+//! Windows cannot execute script files (e.g., `.cmd`, `.bat`) directly through
+//! `Command::new()` without their file extensions, while Unix systems handle
+//! scripts natively through shebangs.
+//!
+//! The `resolve` function abstracts these platform differences:
+//! - On Unix: Returns the program unchanged (OS handles script execution)
+//! - On Windows: Uses the `which` crate to resolve full paths including extensions
+
+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> {
+    Ok(program)
+}
+
+/// Resolves a program to its executable path on Windows systems.
+///
+/// Windows requires explicit file extensions for script execution. This function
+/// uses the `which` crate to search the `PATH` environment variable and find
+/// the full path to the executable, including necessary script extensions
+/// (`.cmd`, `.bat`, etc.) defined in `PATHEXT`.
+///
+/// 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> {
+    // 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");
+
+    // Attempt resolution via which crate
+    match which::which_in(&program, search_path, &cwd) {
+        Ok(resolved) => {
+            debug!("Resolved {:?} to {:?}", program, resolved);
+            Ok(resolved.into_os_string())
+        }
+        Err(e) => {
+            debug!(
+                "Failed to resolve {:?}: {}. Using original path",
+                program, e
+            );
+            // Fallback to original program - let Command::new() handle the error
+            Ok(program)
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::utils::create_env_for_mcp_server;
+    use anyhow::Result;
+    use std::fs;
+    use std::path::Path;
+    use tempfile::TempDir;
+    use tokio::process::Command;
+
+    /// Unix: Verifies the OS handles script execution without file extensions.
+    #[cfg(unix)]
+    #[tokio::test]
+    async fn test_unix_executes_script_without_extension() -> Result<()> {
+        let env = TestExecutableEnv::new()?;
+        let mut cmd = Command::new(&env.program_name);
+        cmd.envs(&env.mcp_env);
+
+        let output = cmd.output().await;
+        assert!(output.is_ok(), "Unix should execute scripts directly");
+        Ok(())
+    }
+
+    /// Windows: Verifies scripts fail to execute without the proper extension.
+    #[cfg(windows)]
+    #[tokio::test]
+    async fn test_windows_fails_without_extension() -> Result<()> {
+        let env = TestExecutableEnv::new()?;
+        let mut cmd = Command::new(&env.program_name);
+        cmd.envs(&env.mcp_env);
+
+        let output = cmd.output().await;
+        assert!(
+            output.is_err(),
+            "Windows requires .cmd/.bat extension for direct execution"
+        );
+        Ok(())
+    }
+
+    /// Windows: Verifies scripts with an explicit extension execute correctly.
+    #[cfg(windows)]
+    #[tokio::test]
+    async fn test_windows_succeeds_with_extension() -> Result<()> {
+        let env = TestExecutableEnv::new()?;
+        // Append the `.cmd` extension to the program name
+        let program_with_ext = format!("{}.cmd", env.program_name);
+        let mut cmd = Command::new(&program_with_ext);
+        cmd.envs(&env.mcp_env);
+
+        let output = cmd.output().await;
+        assert!(
+            output.is_ok(),
+            "Windows should execute scripts when the extension is provided"
+        );
+        Ok(())
+    }
+
+    /// Verifies program resolution enables successful execution on all platforms.
+    #[tokio::test]
+    async fn test_resolved_program_executes_successfully() -> Result<()> {
+        let env = TestExecutableEnv::new()?;
+        let program = OsString::from(&env.program_name);
+
+        // Apply platform-specific resolution
+        let resolved = resolve(program, &env.mcp_env)?;
+
+        // Verify resolved path executes successfully
+        let mut cmd = Command::new(resolved);
+        cmd.envs(&env.mcp_env);
+        let output = cmd.output().await;
+
+        assert!(
+            output.is_ok(),
+            "Resolved program should execute successfully"
+        );
+        Ok(())
+    }
+
+    // Test fixture for creating temporary executables in a controlled environment.
+    struct TestExecutableEnv {
+        // Held to prevent the temporary directory from being deleted.
+        _temp_dir: TempDir,
+        program_name: String,
+        mcp_env: HashMap<String, String>,
+    }
+
+    impl TestExecutableEnv {
+        const TEST_PROGRAM: &'static str = "test_mcp_server";
+
+        fn new() -> Result<Self> {
+            let temp_dir = TempDir::new()?;
+            let dir_path = temp_dir.path();
+
+            Self::create_executable(dir_path)?;
+
+            // 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));
+
+            #[cfg(windows)]
+            extra_env.insert("PATHEXT".to_string(), Self::ensure_cmd_extension());
+
+            let mcp_env = create_env_for_mcp_server(Some(extra_env), &[]);
+
+            Ok(Self {
+                _temp_dir: temp_dir,
+                program_name: Self::TEST_PROGRAM.to_string(),
+                mcp_env,
+            })
+        }
+
+        /// Creates a simple, platform-specific executable script.
+        fn create_executable(dir: &Path) -> Result<()> {
+            #[cfg(windows)]
+            {
+                let file = dir.join(format!("{}.cmd", Self::TEST_PROGRAM));
+                fs::write(&file, "@echo off\nexit 0")?;
+            }
+
+            #[cfg(unix)]
+            {
+                let file = dir.join(Self::TEST_PROGRAM);
+                fs::write(&file, "#!/bin/sh\nexit 0")?;
+                Self::set_executable(&file)?;
+            }
+
+            Ok(())
+        }
+
+        #[cfg(unix)]
+        fn set_executable(path: &Path) -> Result<()> {
+            use std::os::unix::fs::PermissionsExt;
+            let mut perms = fs::metadata(path)?.permissions();
+            perms.set_mode(0o755);
+            fs::set_permissions(path, perms)?;
+            Ok(())
+        }
+
+        /// 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())
+        }
+
+        /// 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") {
+                current
+            } else {
+                format!(".CMD;{current}")
+            }
+        }
+    }
+}

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

@@ -47,6 +47,7 @@ use crate::logging_client_handler::LoggingClientHandler;
 use crate::oauth::OAuthCredentialsStoreMode;
 use crate::oauth::OAuthPersistor;
 use crate::oauth::StoredOAuthTokens;
+use crate::program_resolver;
 use crate::utils::apply_default_headers;
 use crate::utils::build_default_headers;
 use crate::utils::convert_call_tool_result;
@@ -91,13 +92,20 @@ impl RmcpClient {
         cwd: Option<PathBuf>,
     ) -> io::Result<Self> {
         let program_name = program.to_string_lossy().into_owned();
-        let mut command = Command::new(&program);
+
+        // Build environment for program resolution and subprocess
+        let envs = create_env_for_mcp_server(env, env_vars);
+
+        // Resolve program to executable path (platform-specific)
+        let resolved_program = program_resolver::resolve(program, &envs)?;
+
+        let mut command = Command::new(resolved_program);
         command
             .kill_on_drop(true)
             .stdin(Stdio::piped())
             .stdout(Stdio::piped())
             .env_clear()
-            .envs(create_env_for_mcp_server(env, env_vars))
+            .envs(envs)
             .args(&args);
         if let Some(cwd) = cwd {
             command.current_dir(cwd);