Commit Graph

5 Commits

  • Add turn-scoped environment selections (#18416)
    ## Summary
    - add experimental turn/start.environments params for per-turn
    environment id + cwd selections
    - pass selections through core protocol ops and resolve them with
    EnvironmentManager before TurnContext creation
    - treat omitted selections as default behavior, empty selections as no
    environment, and non-empty selections as first environment/cwd as the
    turn primary
    
    ## Testing
    - ran `just fmt`
    - ran `just write-app-server-schema`
    - not run: unit tests for this stacked PR
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>
  • [5/6] Wire executor-backed MCP stdio (#18212)
    ## Summary
    - Add the executor-backed RMCP stdio transport.
    - Wire MCP stdio placement through the executor environment config.
    - Cover local and executor-backed stdio paths with the existing MCP test
    helpers.
    
    ## Stack
    ```text
    o  #18027 [6/6] Fail exec client operations after disconnect
    │
    @  #18212 [5/6] Wire executor-backed MCP stdio
    │
    o  #18087 [4/6] Abstract MCP stdio server launching
    │
    o  #18020 [3/6] Add pushed exec process events
    │
    o  #18086 [2/6] Support piped stdin in exec process API
    │
    o  #18085 [1/6] Add MCP server environment config
    │
    o  main
    ```
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>
  • fix: keep rmcp-client env vars as OsString (#15363)
    ## Why
    
    This is a follow-up to #15360. That change fixed the `arg0` helper
    setup, but `rmcp-client` still coerced stdio transport environment
    values into UTF-8 `String`s before program resolution and process spawn.
    If `PATH` or another inherited environment value contains non-UTF-8
    bytes, that loses fidelity before it reaches `which` and `Command`.
    
    ## What changed
    
    - change `create_env_for_mcp_server()` to return `HashMap<OsString,
    OsString>` and read inherited values with `std::env::var_os()`
    - change `TransportRecipe::Stdio.env`, `RmcpClient::new_stdio_client()`,
    and `program_resolver::resolve()` to keep stdio transport env values in
    `OsString` form within `rmcp-client`
    - keep the `codex-core` config boundary stringly, but convert configured
    stdio env values to `OsString` once when constructing the transport
    - update the rmcp-client stdio test fixtures and callers to use
    `OsString` env maps
    - add a Unix regression test that verifies `create_env_for_mcp_server()`
    preserves a non-UTF-8 `PATH`
    
    ## How to verify
    
    - `cargo test -p codex-rmcp-client`
    - `cargo test -p codex-core mcp_connection_manager`
    - `just argument-comment-lint`
    
    Targeted coverage in this change includes
    `utils::tests::create_env_preserves_path_when_it_is_not_utf8`, while the
    updated stdio transport path is exercised by the existing rmcp-client
    tests that construct `RmcpClient::new_stdio_client()`.
  • fix: resolve Windows MCP server execution for script-based tools (#3828)
    ## What?
    
    Fixes MCP server initialization failures on Windows when using
    script-based tools like `npx`, `pnpm`, and `yarn` that rely on
    `.cmd`/`.bat` files rather than `.exe` binaries.
    
    Fixes #2945
    
    ## Why?
    
    Windows users encounter "program not found" errors when configuring MCP
    servers with commands like `npx` in their `~/.codex/config.toml`. This
    happens because:
    
    - Tools like `npx` are batch scripts (`npx.cmd`) on Windows, not
    executable binaries
    - Rust's `std::process::Command` bypasses the shell and cannot execute
    these scripts directly
    - The Windows shell normally handles this by checking `PATHEXT` for
    executable extensions
    
    Without this fix, Windows users must specify full paths or add `.cmd`
    extensions manually, which breaks cross-platform compatibility.
    
    ## How?
    
    Added platform-specific program resolution using the `which` crate to
    find the correct executable path:
    
    - **Windows**: Resolves programs through PATH/PATHEXT to find
    `.cmd`/`.bat` scripts
    - **Unix**: Returns the program unchanged (no-op, as Unix handles
    scripts natively)
    
    ### Changes
    
    - Added `which = "6"` dependency to `mcp-client/Cargo.toml`
    - Implemented `program_resolver` module in `mcp_client.rs` with
    platform-specific resolution
    - Added comprehensive tests for both Windows and Unix behavior
    
    ### Testing
    
    Added platform-specific tests to verify:
    - Unix systems execute scripts without extensions
    - Windows fails without proper extensions
    - Windows succeeds with explicit extensions
    - Cross-platform resolution enables successful execution
    
    **Tested on:**
    - Windows 11 (NT 10.0.26100.0 x64)
    - PowerShell 5.1 & 7+, CMD, Git Bash
    - MCP servers: playwright, context7, supabase
    - WSL (verified no regression)
    
    **Local checks passed:**
    ```bash
    cargo test && cargo clippy --tests && cargo fmt -- --config imports_granularity=Item
    ```
    
    ### Results
    
    **Before:**
    ```
    🖐 MCP client for `playwright` failed to start: program not found
    ```
    
    **After:**
    ```
    🖐 MCP client for `playwright` failed to start: request timed out
    ```
    
    Windows users can now use simple commands like `npx` in their config
    without specifying full paths or extensions. The timeout issue is a
    separate concern that will be addressed in a follow-up PR.
    
    ---------
    
    Co-authored-by: Eric Traut <etraut@openai.com>