Commit Graph

21 Commits

  • [codex] trace MCP startup latency (#28630)
    ## Summary
    
    - add trace-level instrumentation around per-server MCP setup, client
    construction, initialization, and initial tool listing
    - trace Codex Apps tool and server-info cache loads
    - attach `server_name` to server-scoped spans so slow startup work can
    be attributed to a specific MCP server
    
    ## Why
    
    `session_init.mcp_manager_init` can occasionally be slow, but its
    existing coarse span does not identify whether time is spent loading the
    Codex Apps cache, constructing a client, initializing a transport, or
    listing tools. These definition-level spans provide that breakdown
    without changing startup behavior.
    
    ## Validation
    
    - `just test -p codex-mcp` (87 passed)
    - `just test -p codex-rmcp-client` (86 passed, 2 skipped)
  • Remove redundant Codex Apps cache guard (#29575)
    ## Why
    
    Codex Apps cache writes are already restricted to Codex Apps call paths:
    startup invokes the helper only from the Codex Apps branch, and hard
    refresh operates on the reserved Codex Apps server directly. Rechecking
    the server name inside the cache helper duplicates that classification
    and leaves the helper with an argument that cannot change valid
    behavior.
    
    ## What changed
    
    - Remove the redundant server-name check and parameter from the cache
    writer.
    - Rename the helper to `write_codex_apps_tools_cache` to reflect its
    narrower contract.
    - Update production and test callsites to use the simplified API.
  • Centralize Codex Apps client handling (#29528)
    ## Why
    
    Codex Apps-specific behavior is currently distributed across cache
    helpers, startup, tool conversion, and model-visible annotation. Each
    layer independently checks the reserved server name, which obscures the
    boundary between trusted host-owned connector metadata and regular MCP
    server data.
    
    Classifying the server once when `AsyncManagedClient` is created gives
    the client a single source of truth and makes the two processing paths
    explicit.
    
    ## What changed
    
    - Record whether an `AsyncManagedClient` represents the Codex Apps
    server at construction time.
    - Route startup cache loading, cache persistence, and cache telemetry
    through the Codex Apps branch.
    - Split uncached tool conversion between Codex Apps normalization and
    regular MCP metadata sanitization.
    - Split model-visible schema and plugin provenance handling along the
    same boundary.
    - Remove redundant server-name guards from helpers that are now called
    only from the Codex Apps branch.
    
    ## Verification
    
    - Preserve behavioral coverage that verifies Codex Apps connector
    metadata and the complete converted `ToolInfo` shape.
    
    ## Stack
    
    Depends on #29518.
  • mcp: accept foreign absolute cwd for remote stdio (#29493)
    ## Why
    
    Remote stdio MCP servers can run in an environment whose path convention
    differs from the Codex host. A Windows cwd such as
    `C:\Users\openai\share` is absolute for the executor but was rejected by
    a POSIX orchestrator.
    
    Built on #29501, now merged, which only clarifies the host-native
    `PathUri` constructor name.
    
    ## What changed
    
    - Deserialize MCP cwd values as `LegacyAppPathString` so config does not
    apply host path rules.
    - Interpret that spelling as host-native for local launches and convert
    it to `PathUri` at executor launch.
    - Skip host filesystem and command resolution checks for remote stdio in
    `codex doctor`.
    - Add host-independent config and executor-boundary coverage using the
    foreign path convention for each test platform.
    
    ## Validation
    
    - `just test -p codex-utils-path-uri -p codex-config -p codex-mcp -p
    codex-rmcp-client` (408 passed)
    - `just test -p codex-cli -p codex-rmcp-client` (372 passed)
    - `cargo check --workspace --tests`
    - `just test` (11,311 passed; 43 unrelated environment/timing failures)
    - `just fix -p codex-cli -p codex-config -p codex-core -p codex-mcp -p
    codex-mcp-extension -p codex-rmcp-client -p codex-tui`
  • [codex] Remove hardcoded app ID filters (#28947)
    ## Summary
    
    - remove the duplicated originator-specific connector ID denylists
    - stop filtering connector directory/accessibility results and
    live/cached Codex Apps MCP tools by hardcoded connector ID
    - remove the now-unused `codex-login` dependency from
    `codex-utils-plugins`
    - update regression coverage so formerly blocked connector IDs are
    preserved
    
    ## Why
    
    The client-side policy was duplicated across crates, used opaque IDs
    without ownership or expiry information, and could drift between app
    listing and MCP tool behavior. Server-provided visibility,
    authorization, plugin discoverability, accessibility, enabled-state
    handling, and consequential-tool approval templates remain unchanged.
    
    ## Validation
    
    - `just fmt`
    - `just bazel-lock-update`
    - `just bazel-lock-check`
    - `git diff --check`
    - confirmed the final diff contains no hardcoded denylist symbols
    
    A targeted `codex-mcp` test build spent an unusually long time in local
    compilation/linking. Its first attempt exposed a test-only `PartialEq`
    assertion issue, which was corrected. A follow-up non-linking `cargo
    check -p codex-mcp --tests` was still running when this draft was
    opened; CI should provide the complete Rust validation.
  • Support openai/form extended form elicitations (#27500)
    # Summary
    Allow App Server clients to opt into `openai/form` MCP elicitations.
  • [mcp] Increase default tool timeout to 300 seconds (#28234)
    Summary
    - Increase the default MCP tool-call timeout from 120 to 300 seconds.
    
    Validation
    - `just test -p codex-mcp`
    - `just fmt`
  • feat: use encrypted local secrets for MCP OAuth (#27541)
    ## Summary
    
    - store MCP OAuth credentials in the configured auth credential backend
    - support encrypted-local OAuth storage, including legacy keyring
    migration
    - propagate the credential backend through MCP refresh, session, CLI,
    and app-server paths
    
    ## Stack
    
    1. #27504 — config and feature flag
    2. #27535 — auth-specific secret namespaces
    3. #27539 — encrypted CLI auth storage
    4. this PR — encrypted MCP OAuth storage
    
    This is a parallel review stack; the original #17931 remains unchanged.
    
    ## Tests
    
    - `just test -p codex-rmcp-client` (the transport round-trip test passed
    after building the required `codex` binary and retrying)
    - `just test -p codex-mcp`
    - `just test -p codex-app-server
    refresh_config_uses_latest_auth_keyring_backend`
    - `just test -p codex-core
    refresh_mcp_servers_is_deferred_until_next_turn`
    - `just test -p codex-cli mcp`
    - `just fix -p codex-rmcp-client -p codex-mcp -p codex-core -p codex-cli
    -p codex-app-server -p codex-protocol`
    - `just bazel-lock-check`
  • Expose MCP server info as part of server status (#24698)
    # Summary
    
    Expose MCP server info via App Server (when available) so apps can
    render a richer MCP experience
  • Update rmcp to 1.7.0 (#24763)
    WIll make it easier to uprev when the new draft spec is supported.
    
    Also updates reqwest where needed for compatibility but doesn't update
    it everywhere since this is already a large diff.
    
    The new version of rmcp handles certain kinds of authentication failures
    differently, this patch includes support for identifying the failing scope
    in a WWW-Authenticate header.
  • Route MCP servers through explicit environments (#23583)
    ## Summary
    - route each configured MCP server through an explicit per-server
    `environment_id` instead of a manager-wide remote toggle
    - default omitted `environment_id` to `local`, resolve named ids through
    `EnvironmentManager`, and fail only the affected MCP server when an
    explicit id is unknown
    - keep local stdio on the existing local launcher path for now, while
    named-environment stdio uses the selected environment backend and
    requires an absolute `cwd`
    - allow local HTTP MCP servers to keep using the ambient HTTP client
    when no local `Environment` is configured; named-environment HTTP MCPs
    use that environment's HTTP client
    
    ## Validation
    - devbox Bazel build: `bazel build --bes_backend= --bes_results_url=
    //codex-rs/cli:codex //codex-rs/rmcp-client:test_stdio_server
    //codex-rs/rmcp-client:test_streamable_http_server`
    - devbox app-server config matrix with real `config.toml` /
    `environments.toml` files covering omitted local, explicit local,
    omitted local under remote default, explicit remote stdio, local HTTP
    without local env, explicit remote HTTP, local stdio without local env,
    unknown explicit env, and remote stdio without `cwd`
  • Make local environment optional in EnvironmentManager (#23369)
    ## Summary
    - make `EnvironmentManager` local environment/runtime paths optional
    - simplify constructor surface around snapshot materialization
    - rename local env accessors to `require_local_environment` /
    `try_local_environment`
    
    ## Validation
    - devbox Bazel build for touched crate surfaces
    - `//codex-rs/exec-server:exec-server-unit-tests`
    - `//codex-rs/app-server-client:app-server-client-unit-tests`
    - filtered touched `//codex-rs/core:core-unit-tests` cases
  • Simplify MCP tool handler plumbing (#21595)
    ## Why
    The MCP tool path had accumulated a few core-owned special cases: a
    dedicated payload variant, resolver plumbing, a legacy `AfterToolUse`
    translation path, and a side channel for parallel-call metadata. That
    made `ToolRegistry` and the spec builder know more about MCP than they
    needed to.
    
    This change moves MCP-specific execution details back onto `ToolInfo`
    and `McpHandler` so `codex-core` can treat MCP calls like normal
    function calls while still preserving MCP-specific dispatch and
    telemetry behavior where it belongs.
    
    ## What changed
    - removed `resolve_mcp_tool_info`, `ToolPayload::Mcp`, `ToolKind`, and
    the remaining registry-side MCP resolver path
    - stored MCP routing metadata directly on `McpHandler` and `ToolInfo`,
    including `supports_parallel_tool_calls`
    - deleted the legacy `AfterToolUse` consumer in `core`, which removes
    the need for handler-specific `after_tool_use_payload` implementations
    - switched tool-result telemetry to handler-provided tags and kept
    MCP-specific dispatch payload construction inside the handler
    - simplified tool spec planning/building by passing `ToolInfo` directly
    and dropping the direct/deferred MCP wrapper structs and the
    parallel-server side table
    
    ## Testing
    - `cargo check -p codex-core -p codex-mcp -p codex-otel`
    - `cargo test -p codex-core
    mcp_parallel_support_uses_exact_payload_server`
    - `cargo test -p codex-core
    direct_mcp_tools_register_namespaced_handlers`
    - `cargo test -p codex-core
    search_tool_description_lists_each_mcp_source_once`
    - `cargo test -p codex-mcp
    list_all_tools_uses_startup_snapshot_while_client_is_pending`
    - `just fix -p codex-core -p codex-mcp -p codex-otel`
  • [elicitation] Advertise new url elicitation capability when auth_elicitation is enabled. (#22188)
    ## Why
    
    We've added support for auth elicitation behind the auth_elicitation
    flag, but servers need to explicitly check the capability before it
    decides to send elicitations in order to be backward compatible. This PR
    adds the capability advertising conditioned on the flag.
    
    ## What changed
    
    - Build `client_elicitation_capability` from the `AuthElicitation`
    feature state.
    - Thread that capability through MCP config, session startup, and
    `McpConnectionManager` so RMCP initialization advertises the correct
    elicitation support.
    - Advertise both `form` and `url` elicitation when the feature is
    enabled, and preserve the empty default capability when it is disabled.
    - Add coverage for the feature-derived config shape and the advertised
    initialization payload.
    
    ## Testing
    
    - `cargo test -p codex-mcp`
    - `cargo test -p codex-core
    to_mcp_config_preserves_auth_elicitation_feature_from_config`
    - `cargo test -p codex-core` *(currently fails outside this change in
    `tools::handlers::multi_agents::tests::tool_handlers_cascade_close_and_resume_and_keep_explicitly_closed_subtrees_closed`
    with a stack overflow after unrelated tests have started running)*
  • chore: drop built-in MCPs (#22173)
    Drop something that was never used
  • feat: make built-in MCPs first-class runtime servers (#21356)
    ## DISCLAIMER
    This is experimental and no production service must rely on this
    
    ## Why
    
    Built-in MCPs are product-owned runtime capabilities, but they were
    previously flattened into the same config-backed stdio path as
    user-configured servers. That made them depend on a hidden `codex
    builtin-mcp` re-exec path, exposed them through config-oriented CLI
    flows, and erased distinctions the runtime needs to preserve—most
    notably whether an MCP call should count as external context for
    memory-mode pollution.
    
    ## What changed
    
    - Model product-owned built-ins separately from config-backed MCP
    servers via `BuiltinMcpServer` and `EffectiveMcpServer`.
    - Launch built-ins in process through a reusable async transport instead
    of the hidden `builtin-mcp` stdio subcommand.
    - Keep config-oriented CLI operations such as `codex mcp
    list/get/login/logout` scoped to configured servers, while merging
    built-ins only into the effective runtime server set.
    - Retain server metadata after launch so parallel-tool support and
    context classification come from the live server set; built-in
    `memories` is now classified as local Codex state rather than external
    context.
    
    ## Test plan
    
    - `cargo test -p codex-mcp`
    - `cargo test -p codex-core --test suite
    builtin_memories_mcp_call_does_not_mark_thread_memory_mode_polluted_when_configured`
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>
  • Use MCP server instructions in deferred namespace descriptions (#21053)
    ## Why
    
    MCP servers can provide `instructions` that explain what their tools are
    for. Directly exposed MCP namespaces already use those instructions when
    a connector description is not available, but deferred `tool_search`
    results did not preserve that fallback. The direct path falls back from
    connector metadata to server instructions, while the deferred path only
    carried `connector_description` and otherwise fell back to generic
    namespace text.
    
    That meant a plain MCP server could provide useful model-facing guidance
    and still appear as `Tools in the X namespace.` whenever it was
    discovered lazily through `tool_search`.
    
    ## What changed
    
    - Store one model-facing `namespace_description` on `ToolInfo`, using
    connector descriptions for connector-backed tools and server
    instructions for plain MCP servers.
    - Thread that namespace description through the `tool_search` source
    list, search indexing, and returned namespace metadata.
    - Add an end-to-end regression test for deferred non-app MCP search
    results exposing server instructions as the namespace description.
    
    ## Verification
    
    - `cargo test -p codex-tools
    search_tool_description_lists_each_mcp_source_once --lib`
    - `cargo test -p codex-core --test all
    tool_search_uses_non_app_mcp_server_instructions_as_namespace_description`
  • Use the 2025-06-18 elicitation capability shape (#20562)
    # Why
    
    Codex currently negotiates MCP `2025-06-18`, where the client
    elicitation capability is represented as an empty object. We were still
    serializing `capabilities.elicitation.form`, which belongs to the later
    capability shape and can cause strict `2025-06-18` servers to reject
    `initialize` with an unrecognized-field error.
    
    This keeps the handshake aligned with the protocol version Codex
    actually negotiates and fixes the compatibility regression tracked in
    #17492.
    
    # What
    
    - Serialize the client elicitation capability as `elicitation: {}` for
    `2025-06-18`.
    - Keep elicitation advertised for both Codex Apps and custom MCP
    servers.
    - Tighten regression coverage so the unit test asserts both the Rust
    value and the serialized wire shape.
    - Add an app-server integration test that round-trips a form elicitation
    from a custom MCP server; the existing connector round-trip continues to
    cover the connector path.
    
    # Verification
    
    - `cargo test -p codex-mcp`
    - `cargo test -p codex-app-server mcp_server_elicitation_round_trip`
    - `cargo test -p codex-app-server
    mcp_server_tool_call_round_trips_elicitation`
    
    # Next steps
    
    - Decide whether `tool_call_mcp_elicitation=false` should also suppress
    capability advertisement during `initialize`.
    - Revisit `form` / `url` capability advertisement when Codex is ready to
    negotiate MCP `2025-11-25`, which defines that newer shape.
  • Strip connector provenance metadata from custom MCP tools (#19875)
    # Summary 
    This prevents non-codex_apps MCP servers from spoofing connector
    provenance metadata.
  • Terminate stdio MCP servers on shutdown to avoid process leaks (#19753)
    ## Why
    
    Several bug reports describe thread shutdown (including subagent
    threads) leaving stdio MCP server processes behind. These reports all
    point at the same lifecycle gap: Codex launches stdio MCP servers, but
    the session-level shutdown path does not explicitly close MCP clients or
    terminate the server process tree.
    
    Fixes #12491
    Fixes #12976
    Fixes #18881
    Fixes #19469
    
    ## History
    
    This is best understood as a regression/coverage gap in MCP session
    lifecycle management, not as stdio MCP cleanup being absent all along.
    #10710 added process-group cleanup for stdio MCP servers, but that
    cleanup only runs when the `RmcpClient`/transport is dropped. The older
    reports (#12491 and #12976) came after that cleanup existed, which
    suggests the remaining problem was that some higher-level shutdown paths
    kept the MCP manager alive or replaced it without explicitly draining
    clients. The newer reports (#18881 and #19469) exposed the same family
    around manager replacement and shutdown.
    
    ## What changed
    
    - Added an explicit stdio MCP process handle in `codex-rmcp-client` so
    local MCP servers terminate their process group and executor-backed MCP
    servers call the executor process terminator.
    - Added `RmcpClient::shutdown()` and manager-level MCP shutdown draining
    so session shutdown, channel-close fallback, MCP refresh, and connector
    probing stop owned MCP clients.
    - Added regression coverage that starts a stdio MCP server, begins an
    in-flight blocking tool call, shuts down the client, and asserts the
    server process exits.
    
    ## Verification
    
    - `cargo test -p codex-rmcp-client`
    - `cargo test -p codex-mcp`
    - `just fix -p codex-rmcp-client`
    - `just fix -p codex-mcp`
    - `just fix -p codex-core`
    
    - Manual before/after validation with a temporary repro script:
    - Pre-fix binary from `HEAD^` (`fed0a8f4fa`): reproduced the leak with
    surviving MCP server and child PIDs, `survivors=[77583, 77592]`,
    `leaked=true`.
    - Post-fix binary from this branch (`67e318148b`): verified both MCP
    processes were gone after interrupting `codex exec`, `survivors=[]`,
    `leaked=false`.
  • Split MCP connection modules (#19725)
    ## Why
    
    The MCP connection manager module had grown to mix orchestration, RMCP
    client startup, elicitation handling, Codex Apps cache and naming
    behavior, tool qualification and filtering, and runtime data. The
    previous stacked PRs split these responsibilities incrementally; this PR
    collapses that work into one self-contained refactor on latest main.
    
    ## What changed
    
    - Move McpConnectionManager into connection_manager.rs.
    - Move RMCP client lifecycle, startup, and uncached tool listing into
    rmcp_client.rs.
    - Move elicitation request tracking and policy handling into
    elicitation.rs.
    - Move Codex Apps cache, key, filtering, and naming helpers into
    codex_apps.rs.
    - Rename the tool-name helper module to tools.rs and move ToolInfo, tool
    filtering, schema masking, and qualification there.
    - Move runtime and sandbox shared types into runtime.rs.
    - Preserve latest main PermissionProfile-based MCP elicitation
    auto-approval behavior.
    
    ## Verification
    
    - just fmt
    - cargo check -p codex-mcp
    - cargo check -p codex-mcp --tests
    - cargo check -p codex-core
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>