fix(windows-path): preserve PATH order; include core env vars #5579

cpjet64 · 2025-10-23T21:58:53Z

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: MCP servers fail to start on Windows due to missing inherited environment variables #4180 prepend_path_entry_for_apply_patch mangles $PATH and interferes with python virtualenv behavior #5225 MCP client for playwright failed to start: program not found #2945 Windows can not launch MCP servers using PNPM #3245 The solution for encountering the Error "Error creating local task: Error: Timeout" when using "startup_timeout_ms" under windows #3385 Unable to execute shell commands #2892 Playwright mcp fails on windows : program not found #3310 Vs Code Extension MCP timed out and program not found #3457 Documentation for Docker Desktop MCP Gateway #4370
Supersedes: fix(mcp): preserve Windows env vars for MCP servers (#4180) #4182, fix(mcp): resolve Windows MCP startup timeout by adding essential environment variables #3866, fix: resolve Windows MCP server execution for script-based tools #3828 (overlapping/inferior once this merges)
Notes: [apply-patch] Add binary to path #2626 / feat: use the arg0 trick with apply_patch #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

Append (not prepend) helper dir to PATH in Rust and Node wrappers
Windows MCP child inherits core env vars; PATH mirrored to Path
cargo fmt / clippy clean across touched crates
Unit tests updated/passing where applicable
Cross-platform behavior preserved (macOS/Linux PATH precedence intact)

Context: * Address open issues related to PATH/PATHEXT and Windows env propagation. * Prepending to PATH changed users' tool precedence (e.g., virtualenv). MCP servers on Windows missed critical env vars (COMSPEC, SYSTEMROOT, etc.). Change: * Append apply_patch temp dir to PATH instead of prepending (arg0, Node wrapper). * Expand Windows DEFAULT_ENV_VARS for MCP client to include COMSPEC, SYSTEMROOT, PROGRAMFILES, APPDATA, etc. Risk: * Low: only affects PATH ordering (preserved) and adds allowed env passthrough. No privilege broadening. Tests: * Existing rmcp-client unit tests pass. Verification: * Ran cargo fmt. * Built and ran tests: `cargo test -p codex-rmcp-client` (all passed). `cargo check -p codex-arg0` OK.

Context: * Some Windows programs (incl. MCP servers/plugins) read canonical `Path` key. * Users had to manually set PWSH/POWERSHELL; include when present. Change: * In rmcp-client env builder, ensure `Path` mirrors `PATH` on Windows. * Add PROGRAMW6432, POWERSHELL, PWSH to the default pass-through list. Risk: * Low; only adds env keys when available. No privilege expansion. Tests: * Added Windows unit test verifying PATH/Path aliasing. Verification: * cargo fmt; cargo clippy -p codex-rmcp-client; cargo test -p codex-rmcp-client (all ok).

- Conditionally declare `mut` only on Windows in env builder. - Remove unnecessary `clone()` flagged by clippy. Validated with: cargo clippy -p codex-rmcp-client -D warnings.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

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

- On Windows, if only `Path` is provided (extra env/whitelist), ensure `PATH` is set to the same value (and vice versa). Avoids HashMap ordering races. Validated: clippy clean, rmcp-client tests pass.

dylan-hurd-oai

Commenting to say that this change feels reasonable to me, but am going to test this before approving

cpjet64 · 2025-10-28T17:02:08Z

Commenting to say that this change feels reasonable to me, but am going to test this before approving

I have good availability today so if there are adjustments you want let me know and I can work on them ASAP. I want to get Windows up to where it needs to be. This goes for all of my PRs.

dylan-hurd-oai · 2025-10-29T00:39:47Z

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

    "TMP",
+    // Common shells/pwsh hints
+    "POWERSHELL",
+    "PWSH",


@cpjet64 if we separate these changes, i'm happy to stamp the DEFAULT_ENV_VARS change and merge it

@dylan-hurd-oai Happy to do so but was hoping it would pass since my next PR for this issue is virtual shells like Python venv and such and the non-PATH fix is the foundation.

I will make this PR into a dedicated path fix in the meantime.

Thank you!

I'm not opposed to switching to appending here, but the impacts of the path change are just more subtle, and might impact the success rate of apply_patch calls. Adding env vars to unblock better MCP support on windows feels much lower risk, so let's get that improvement in and we can focus on this PATH change here.

@dylan-hurd-oai Understood and I agree the env var change is definitely more low risk because I didn't consider the apply-patch impact in that manner. I'll split them now and get you a new commit in a few minutes! Later I'll dig more into apply_patch to make sure before I push a new PR for it. Thanks for that!

…:\n* Reviewer requested separating concerns; keep env passthrough low-risk changes in this PR.\n\nChange:\n* Revert PATH precedence changes in arg0 and Node wrapper (restore prepend).\n* Remove PATH<->Path aliasing logic and test from rmcp-client; retain expanded DEFAULT_ENV_VARS.\n\nRisk:\n* None beyond reverting to upstream behavior for PATH order and aliasing; env list expansion preserved.\n\nTests:\n* Ran just fmt; just fix -p codex-arg0, codex-rmcp-client.\n* cargo test -p codex-rmcp-client -> all tests pass.\n\nVerification:\n* Diff for PR now limited to DEFAULT_ENV_VARS in rmcp-client.

cpjet64 · 2025-10-29T01:22:35Z

Update: Split the changes as requested.

Kept this PR focused on Windows env passthrough only: expanded DEFAULT_ENV_VARS in codex-rs/rmcp-client/src/utils.rs. Removed the PATH/Path aliasing and reverted PATH ordering changes.
Moved PATH precedence changes to a new PR: Windows: append helper dir to PATH (split from #5579; no env var changes) #5898 (append helper dir to PATH in arg0 + Node wrapper).

This should make #5579 low‑risk and ready for review/merge. If you’d like the PATH<->Path mirroring pursued as a separate follow‑up as well, I can open a dedicated PR for that too.

cpjet64 · 2025-10-29T01:35:23Z

Update: Split the changes as requested.

Kept this PR focused on Windows env passthrough only: expanded DEFAULT_ENV_VARS in codex-rs/rmcp-client/src/utils.rs. Removed the PATH/Path aliasing and reverted PATH ordering changes.

Moved PATH precedence changes to a new PR: Windows: append helper dir to PATH (split from #5579; no env var changes) #5898 (append helper dir to PATH in arg0 + Node wrapper).

This should make #5579 low‑risk and ready for review/merge. If you’d like the PATH<->Path mirroring pursued as a separate follow‑up as well, I can open a dedicated PR for that too.

@dylan-hurd-oai my local bugged out do not merge that. Not everything was committed. Fixing now.

cpjet64 · 2025-10-29T01:41:05Z

Update: Split the changes as requested.

Kept this PR focused on Windows env passthrough only: expanded DEFAULT_ENV_VARS in codex-rs/rmcp-client/src/utils.rs. Removed the PATH/Path aliasing and reverted PATH ordering changes.

Moved PATH precedence changes to a new PR: Windows: append helper dir to PATH (split from #5579; no env var changes) #5898 (append helper dir to PATH in arg0 + Node wrapper).

This should make #5579 low‑risk and ready for review/merge. If you’d like the PATH<->Path mirroring pursued as a separate follow‑up as well, I can open a dedicated PR for that too.

@dylan-hurd-oai my local bugged out do not merge that. Not everything was committed. Fixing now.

OK. BLUF. It was a browser cache issue and everything did actually commit properly GH just wasnt showing me the correct diff. We are good to go pending CI. Terrifying and embarrassing moment for me 😨 !

cpjet64 added 3 commits October 23, 2025 17:20

[chore][lint]: avoid unused_mut on non-Windows; drop redundant clone

2fcff53

- Conditionally declare `mut` only on Windows in env builder. - Remove unnecessary `clone()` flagged by clippy. Validated with: cargo clippy -p codex-rmcp-client -D warnings.

chatgpt-codex-connector bot reviewed Oct 23, 2025

View reviewed changes

codex-rs/rmcp-client/src/utils.rs Outdated Show resolved Hide resolved

[fix][windows-path]: mirror Path <-> PATH consistently

e2d234f

- On Windows, if only `Path` is provided (extra env/whitelist), ensure `PATH` is set to the same value (and vice versa). Avoids HashMap ordering races. Validated: clippy clean, rmcp-client tests pass.

This was referenced Oct 24, 2025

fix(tui/win): keep paste mode across multi-read bursts; adaptive guar… #5585

Open

fix[apply-patch]: preserve original CRLF/LF line endings on text updates #5587

Open

cpjet64 added 9 commits October 23, 2025 23:52

Merge branch 'main' into feat/winpath-pathex

f9787a3

Merge branch 'main' into feat/winpath-pathex

b5eba7c

Merge branch 'main' into feat/winpath-pathex

185cff5

Merge branch 'main' into feat/winpath-pathex

9266740

Merge branch 'main' into feat/winpath-pathex

5a2b5db

Merge branch 'main' into feat/winpath-pathex

e7a1f68

Merge branch 'main' into feat/winpath-pathex

28c5630

Merge branch 'main' into feat/winpath-pathex

ad54e6b

Merge branch 'main' into feat/winpath-pathex

655fdb7

dylan-hurd-oai reviewed Oct 28, 2025

View reviewed changes

dylan-hurd-oai reviewed Oct 29, 2025

View reviewed changes

cpjet64 and others added 2 commits October 28, 2025 21:04

Merge branch 'main' into feat/winpath-pathex

9a08f38

cpjet64 mentioned this pull request Oct 29, 2025

Windows: append helper dir to PATH (split from #5579; no env var changes) #5898

Open

Merge branch 'main' into feat/winpath-pathex

a660432

dylan-hurd-oai approved these changes Oct 29, 2025

View reviewed changes

dylan-hurd-oai merged commit e9135fa into openai:main Oct 29, 2025
25 checks passed

github-actions bot locked and limited conversation to collaborators Oct 29, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(windows-path): preserve PATH order; include core env vars #5579

fix(windows-path): preserve PATH order; include core env vars #5579

cpjet64 commented Oct 23, 2025 •

edited

Loading

chatgpt-codex-connector bot left a comment

Uh oh!

dylan-hurd-oai left a comment

cpjet64 commented Oct 28, 2025

dylan-hurd-oai Oct 29, 2025

cpjet64 Oct 29, 2025

dylan-hurd-oai Oct 29, 2025

cpjet64 Oct 29, 2025 •

edited

Loading

cpjet64 commented Oct 29, 2025

cpjet64 commented Oct 29, 2025 •

edited

Loading

cpjet64 commented Oct 29, 2025

Uh oh!

Labels

2 participants

fix(windows-path): preserve PATH order; include core env vars #5579

fix(windows-path): preserve PATH order; include core env vars #5579

Conversation

cpjet64 commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Preserve PATH precedence & fix Windows MCP env propagation

Problem & intent

Before / After

Scope of change

codex-rs/arg0/src/lib.rs

codex-cli/bin/codex.js

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

Security effects

Testing evidence

Checklist

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

dylan-hurd-oai left a comment

Choose a reason for hiding this comment

cpjet64 commented Oct 28, 2025

dylan-hurd-oai Oct 29, 2025

Choose a reason for hiding this comment

cpjet64 Oct 29, 2025

Choose a reason for hiding this comment

dylan-hurd-oai Oct 29, 2025

Choose a reason for hiding this comment

cpjet64 Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

cpjet64 commented Oct 29, 2025

cpjet64 commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

cpjet64 commented Oct 29, 2025

Uh oh!

Labels

2 participants

cpjet64 commented Oct 23, 2025 •

edited

Loading

`codex-rs/arg0/src/lib.rs`

`codex-cli/bin/codex.js`

`codex-rs/rmcp-client/src/utils.rs`

cpjet64 Oct 29, 2025 •

edited

Loading

cpjet64 commented Oct 29, 2025 •

edited

Loading