From e9135fa7c58460c2fd72105e69e28defaa3136d3 Mon Sep 17 00:00:00 2001 From: Curt <38527447+cpjet64@users.noreply.github.com> Date: Wed, 29 Oct 2025 00:06:39 -0400 Subject: [PATCH] fix(windows-path): preserve PATH order; include core env vars (#5579) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Preserve PATH precedence & fix Windows MCP env propagation ## Problem & intent Preserve user PATH precedence and reduce Windows setup friction for MCP servers by avoiding PATH reordering and ensuring Windows child processes receive essential env vars. - Addresses: #4180 #5225 #2945 #3245 #3385 #2892 #3310 #3457 #4370 - Supersedes: #4182, #3866, #3828 (overlapping/inferior once this merges) - Notes: #2626 / #2646 are the original PATH-mutation sources being corrected. --- ## Before / After **Before** - PATH was **prepended** with an `apply_patch` helper dir (Rust + Node wrapper), reordering tools and breaking virtualenvs/shims on macOS/Linux. - On Windows, MCP servers missed core env vars and often failed to start without explicit per-server env blocks. **After** - Helper dir is **appended** to PATH (preserves user/tool precedence). - Windows MCP child env now includes common core variables and mirrors `PATH` → `Path`, so typical CLIs/plugins work **without** per-server env blocks. --- ## Scope of change ### `codex-rs/arg0/src/lib.rs` - Append temp/helper dir to `PATH` instead of prepending. ### `codex-cli/bin/codex.js` - Mirror the same append behavior for the Node wrapper. ### `codex-rs/rmcp-client/src/utils.rs` - Expand Windows `DEFAULT_ENV_VARS` (e.g., `COMSPEC`, `SYSTEMROOT`, `PROGRAMFILES*`, `APPDATA`, etc.). - Mirror `PATH` → `Path` for Windows child processes. - Small unit test; conditional `mut` + `clippy` cleanup. --- ## Security effects No broadened privileges. Only environment propagation for well-known Windows keys on stdio MCP child processes. No sandbox policy changes and no network additions. --- ## Testing evidence **Static** - `cargo fmt` - `cargo clippy -p codex-arg0 -D warnings` → **clean** - `cargo clippy -p codex-rmcp-client -D warnings` → **clean** - `cargo test -p codex-rmcp-client` → **13 passed** **Manual** - Local verification on Windows PowerShell 5/7 and WSL (no `unused_mut` warnings on non-Windows targets). --- ## Checklist - [x] Append (not prepend) helper dir to PATH in Rust and Node wrappers - [x] Windows MCP child inherits core env vars; `PATH` mirrored to `Path` - [x] `cargo fmt` / `clippy` clean across touched crates - [x] Unit tests updated/passing where applicable - [x] Cross-platform behavior preserved (macOS/Linux PATH precedence intact) --- codex-rs/rmcp-client/src/utils.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/codex-rs/rmcp-client/src/utils.rs b/codex-rs/rmcp-client/src/utils.rs index 17e050cb..8deb4d40 100644 --- a/codex-rs/rmcp-client/src/utils.rs +++ b/codex-rs/rmcp-client/src/utils.rs @@ -171,13 +171,33 @@ pub(crate) const DEFAULT_ENV_VARS: &[&str] = &[ #[cfg(windows)] pub(crate) const DEFAULT_ENV_VARS: &[&str] = &[ + // Core path resolution "PATH", "PATHEXT", + // Shell and system roots + "COMSPEC", + "SYSTEMROOT", + "SYSTEMDRIVE", + // User context and profiles "USERNAME", "USERDOMAIN", "USERPROFILE", + "HOMEDRIVE", + "HOMEPATH", + // Program locations + "PROGRAMFILES", + "PROGRAMFILES(X86)", + "PROGRAMW6432", + "PROGRAMDATA", + // App data and caches + "LOCALAPPDATA", + "APPDATA", + // Temp locations "TEMP", "TMP", + // Common shells/pwsh hints + "POWERSHELL", + "PWSH", ]; #[cfg(test)]