Commit Graph

15 Commits

  • fix(windows-path): preserve PATH order; include core env vars (#5579)
    # 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)
  • [MCP] Render MCP tool call result images to the model (#5600)
    It's pretty amazing we have gotten here without the ability for the
    model to see image content from MCP tool calls.
    
    This PR builds off of 4391 and fixes #4819. I would like @KKcorps to get
    adequete credit here but I also want to get this fix in ASAP so I gave
    him a week to update it and haven't gotten a response so I'm going to
    take it across the finish line.
    
    
    This test highlights how absured the current situation is. I asked the
    model to read this image using the Chrome MCP
    <img width="2378" height="674" alt="image"
    src="https://github.com/user-attachments/assets/9ef52608-72a2-4423-9f5e-7ae36b2b56e0"
    />
    
    After this change, it correctly outputs:
    > Captured the page: image dhows a dark terminal-style UI labeled
    `OpenAI Codex (v0.0.0)` with prompt `model: gpt-5-codex medium` and
    working directory `/codex/codex-rs`
    (and more)  
    
    Before this change, it said:
    > Took the full-page screenshot you asked for. It shows a long,
    horizontally repeating pattern of stylized people in orange, light-blue,
    and mustard clothing, holding hands in alternating poses against a white
    background. No text or other graphics-just rows of flat illustration
    stretching off to the right.
    
    Without this change, the Figma, Playwright, Chrome, and other visual MCP
    servers are pretty much entirely useless.
    
    I tested this change with the openai respones api as well as a third
    party completions api
  • [Auth] Add keyring support for Codex CLI (#5591)
    Follow-up PR to #5569. Add Keyring Support for Auth Storage in Codex CLI
    as well as a hybrid mode (default to persisting in keychain but fall
    back to file when unavailable.)
    
    It also refactors out the keyringstore implementation from rmcp-client
    [here](https://github.com/openai/codex/blob/main/codex-rs/rmcp-client/src/oauth.rs)
    to a new keyring-store crate.
    
    There will be a follow-up that picks the right credential mode depending
    on the config, instead of hardcoding `AuthCredentialsStoreMode::File`.
  • [MCP] Add support for specifying scopes for MCP oauth (#5487)
    ```
    codex mcp login server_name --scopes=scope1,scope2,scope3
    ```
    
    Fixes #5480
  • [MCP] Add support for resources (#5239)
    This PR adds support for [MCP
    resources](https://modelcontextprotocol.io/specification/2025-06-18/server/resources)
    by adding three new tools for the model:
    1. `list_resources`
    2. `list_resource_templates`
    3. `read_resource`
    
    These 3 tools correspond to the [three primary MCP resource protocol
    messages](https://modelcontextprotocol.io/specification/2025-06-18/server/resources#protocol-messages).
    
    Example of listing and reading a GitHub resource tempalte
    <img width="2984" height="804" alt="CleanShot 2025-10-15 at 17 31 10"
    src="https://github.com/user-attachments/assets/89b7f215-2e2a-41c5-90dd-b932ac84a585"
    />
    
    `/mcp` with Figma configured
    <img width="2984" height="442" alt="CleanShot 2025-10-15 at 18 29 35"
    src="https://github.com/user-attachments/assets/a7578080-2ed2-4c59-b9b4-d8461f90d8ee"
    />
    
    Fixes #4956
  • [MCP] Allow specifying cwd and additional env vars (#5246)
    This makes stdio mcp servers more flexible by allowing users to specify
    the cwd to run the server command from and adding additional environment
    variables to be passed through to the server.
    
    Example config using the test server in this repo:
    ```toml
    [mcp_servers.test_stdio]
    cwd = "/Users/<user>/code/codex/codex-rs"
    command = "cargo"
    args = ["run", "--bin", "test_stdio_server"]
    env_vars = ["MCP_TEST_VALUE"]
    ```
    
    @bolinfest I know you hate these env var tests but let's roll with this
    for now. I may take a stab at the env guard + serial macro at some
    point.
  • [MCP] Allow specifying custom headers with streamable http servers (#5241)
    This adds two new config fields to streamable http mcp servers:
    `http_headers`: a map of key to value
    `env_http_headers` a map of key to env var which will be resolved at
    request time
    
    All headers will be passed to all MCP requests to that server just like
    authorization headers.
    
    There is a test ensuring that headers are not passed to other servers.
    
    Fixes #5180
  • [MCP] Prompt mcp login when adding a streamable HTTP server that supports oauth (#5193)
    1. If Codex detects that a `codex mcp add -url …` server supports oauth,
    it will auto-initiate the login flow.
    2. If the TUI starts and a MCP server supports oauth but isn't logged
    in, it will give the user an explicit warning telling them to log in.
  • [MCP] Add auth status to MCP servers (#4918)
    This adds a queryable auth status for MCP servers which is useful:
    1. To determine whether a streamable HTTP server supports auth or not
    based on whether or not it supports RFC 8414-3.2
    2. Allow us to build a better user experience on top of MCP status
  • [MCP] Add the ability to explicitly specify a credentials store (#4857)
    This lets users/companies explicitly choose whether to force/disallow
    the keyring/fallback file storage for mcp credentials.
    
    People who develop with Codex will want to use this until we sign
    binaries or else each ad-hoc debug builds will require keychain access
    on every build. I don't love this and am open to other ideas for how to
    handle that.
    
    
    ```toml
    mcp_oauth_credentials_store = "auto"
    mcp_oauth_credentials_store = "file"
    mcp_oauth_credentials_store = "keyrung"
    ```
    Defaults to `auto`
  • [MCP] Fix the bearer token authorization header (#4846)
    `http_config.auth_header` automatically added `Bearer `. By adding it
    ourselves, we were sending `Bearer Bearer <token>`.
    
    I confirmed that the GitHub MCP initialization 400s before and works
    now.
    
    I also optimized the oauth flow to not check the keyring if you
    explicitly pass in a bearer token.
  • [MCP] Upgrade rmcp to 0.8 (#4774)
    The version with the well-known discovery and my MCP client name change
    were just released
    
    https://github.com/modelcontextprotocol/rust-sdk/releases
  • [MCP] Add support for MCP Oauth credentials (#4517)
    This PR adds oauth login support to streamable http servers when
    `experimental_use_rmcp_client` is enabled.
    
    This PR is large but represents the minimal amount of work required for
    this to work. To keep this PR smaller, login can only be done with
    `codex mcp login` and `codex mcp logout` but it doesn't appear in `/mcp`
    or `codex mcp list` yet. Fingers crossed that this is the last large MCP
    PR and that subsequent PRs can be smaller.
    
    Under the hood, credentials are stored using platform credential
    managers using the [keyring crate](https://crates.io/crates/keyring).
    When the keyring isn't available, it falls back to storing credentials
    in `CODEX_HOME/.credentials.json` which is consistent with how other
    coding agents handle authentication.
    
    I tested this on macOS, Windows, WSL (ubuntu), and Linux. I wasn't able
    to test the dbus store on linux but did verify that the fallback works.
    
    One quirk is that if you have credentials, during development, every
    build will have its own ad-hoc binary so the keyring won't recognize the
    reader as being the same as the write so it may ask for the user's
    password. I may add an override to disable this or allow
    users/enterprises to opt-out of the keyring storage if it causes issues.
    
    <img width="5064" height="686" alt="CleanShot 2025-09-30 at 19 31 40"
    src="https://github.com/user-attachments/assets/9573f9b4-07f1-4160-83b8-2920db287e2d"
    />
    <img width="745" height="486" alt="image"
    src="https://github.com/user-attachments/assets/9562649b-ea5f-4f22-ace2-d0cb438b143e"
    />
  • [MCP] Add experimental support for streamable HTTP MCP servers (#4317)
    This PR adds support for streamable HTTP MCP servers when the
    `experimental_use_rmcp_client` is enabled.
    
    To set one up, simply add a new mcp server config with the url:
    ```
    [mcp_servers.figma]
    url = "http://127.0.0.1:3845/mcp"
    ```
    
    It also supports an optional `bearer_token` which will be provided in an
    authorization header. The full oauth flow is not supported yet.
    
    The config parsing will throw if it detects that the user mixed and
    matched config fields (like command + bearer token or url + env).
    
    The best way to review it is to review `core/src` and then
    `rmcp-client/src/rmcp_client.rs` first. The rest is tests and
    propagating the `Transport` struct around the codebase.
    
    Example with the Figma MCP:
    <img width="5084" height="1614" alt="CleanShot 2025-09-26 at 13 35 40"
    src="https://github.com/user-attachments/assets/eaf2771e-df3e-4300-816b-184d7dec5a28"
    />
  • [MCP] Introduce an experimental official rust sdk based mcp client (#4252)
    The [official Rust
    SDK](https://github.com/modelcontextprotocol/rust-sdk/tree/57fc428c578a1a3fe851ee0838bf068bda120eb3)
    has come a long way since we first started our mcp client implementation
    5 months ago and, today, it is much more complete than our own
    stdio-only implementation.
    
    This PR introduces a new config flag `experimental_use_rmcp_client`
    which will use a new mcp client powered by the sdk instead of our own.
    
    To keep this PR simple, I've only implemented the same stdio MCP
    functionality that we had but will expand on it with future PRs.
    
    ---------
    
    Co-authored-by: pakrym-oai <pakrym@openai.com>