Commit Graph

3 Commits

  • 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()`.
  • Stabilize RMCP pid file cleanup test (#13881)
    ## What changed
    - The pid-file cleanup test now keeps polling when the pid file exists
    but is still empty.
    - Assertions only proceed once the wrapper has actually written the
    child pid.
    
    ## Why this fixes the flake
    - File creation and pid writing are not atomic as one logical action
    from the test’s point of view.
    - The previous test sometimes won the race and read the file in the tiny
    window after creation but before the pid bytes were flushed.
    - Treating “empty file” as “not ready yet” synchronizes the test on the
    real event we need: the wrapper has finished publishing the child pid.
    
    ## Scope
    - Test-only change.
  • Process-group cleanup for stdio MCP servers to prevent orphan process storms (#10710)
    This PR changes stdio MCP child processes to run in their own process
    group
    * Add guarded teardown in codex-rmcp-client: send SIGTERM to the group
    first, then SIGKILL after a short grace period.
    * Add terminate_process_group helper in process_group.rs.
    * Add Unix regression test in process_group_cleanup.rs to verify wrapper
    + grandchild are reaped on client drop.
    
    Addresses reported MCP process/thread storm: #10581