mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
4fca5c32da22c472277e60efcbbe49a8ea3a1789
10 Commits
-
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`
pakrym-oai ·
2026-05-12 00:11:31 +00:00 -
[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)*
Matthew Zeng ·
2026-05-11 12:23:55 -07:00 -
Remove ToolName display helper (#21465)
## Why `ToolName::display()` made it too easy to flatten tool identity and accidentally compare rendered strings. Tool identity should stay structural until a legacy string boundary actually requires the flattened spelling. ## What - Removes `ToolName::display()` and relies on the existing `Display` impl for messages and errors. - Adds structural ordering for `ToolName` and uses it for sorting/deduping deferred tools. - Carries `ToolName` through tool/sandbox plumbing, flattening only at legacy boundaries such as hook payloads, telemetry tags, and Responses tool names. - Updates MCP normalization tests to assert `ToolName` structure instead of rendered strings. ## Testing - `cargo test -p codex-mcp test_normalize_tools` - `cargo test -p codex-core unavailable_tool` - `just fix -p codex-protocol` - `just fix -p codex-mcp` - `just fix -p codex-core`
pakrym-oai ·
2026-05-08 12:17:48 -07:00 -
[codex] Remove string-keyed MCP tool maps (#21454)
## Summary This PR removes the synthetic `HashMap<String, ToolInfo>` keys from MCP tool discovery. `McpConnectionManager::list_all_tools()` now returns normalized `Vec<ToolInfo>`, and downstream code derives identity from `ToolInfo::canonical_tool_name()`. The motivation is to keep model-visible tool identity on `ToolName`/`ToolInfo` instead of parallel string map keys, so future namespace changes do not have to preserve otherwise-unused lookup keys. ## Changes - Rename the MCP normalization path from `qualify_tools` to `normalize_tools_for_model` and return tool values directly. - Flow MCP tool lists through connectors, plugin injection, router/spec building, code mode, and tool search as vectors/slices. - Keep direct/deferred subtraction local to `mcp_tool_exposure`, using `ToolName` values. - Update tests to compare `ToolName` instances where MCP identity matters. ## Validation - `cargo test -p codex-mcp test_normalize_tools` - `cargo test -p codex-core mcp_tool_exposure` - `cargo test -p codex-core direct_mcp_tools_register_namespaced_handlers` - `cargo test -p codex-core search_tool_registers_namespaced_mcp_tool_aliases` - `just fix -p codex-mcp` - `just fix -p codex-core`
pakrym-oai ·
2026-05-07 10:16:10 -07:00 -
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>
jif-oai ·
2026-05-07 10:36:32 +02:00 -
Route opted-in MCP elicitations through Guardian (#19431)
# Motivation Browser Use origin-access prompts are MCP elicitations, not direct tool-call approval prompts, so they were bypassing the Guardian approval path. We need a generic opt-in that lets eligible MCP elicitations use Guardian when the current turn already routes approvals there. # Description Add a generic elicitation reviewer hook in codex-mcp and wire codex-core to pass a Guardian reviewer callback when creating the MCP connection manager. The reviewer validates explicit mcp_tool_call opt-in metadata, builds a Guardian MCP tool-call review request from server/tool/connector metadata and tool params, and maps Guardian approval, denial, timeout, and cancellation decisions back to MCP elicitation responses. The new option to trigger this in the `_meta` object is: ``` "codex_request_type": "approval_request", ``` # Testing - RUST_MIN_STACK=8388608 NEXTEST_STATUS_LEVEL=leak cargo nextest run --no-fail-fast --cargo-profile ci-test --test-threads 2 - cargo clippy --tests -- -D warnings - cargo fmt -- --config imports_granularity=Item --check - cargo shear - pnpm run format - python3 .github/scripts/verify_cargo_workspace_manifests.py - python3 .github/scripts/verify_tui_core_boundary.py - python3 .github/scripts/verify_bazel_clippy_lints.py - git diff --check
Clark DuVall ·
2026-05-06 19:42:45 +00:00 -
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`
sayan-oai ·
2026-05-04 19:36:07 +00:00 -
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.
Abhinav ·
2026-05-01 14:16:22 -07:00 -
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`.
Eric Traut ·
2026-04-28 09:29:57 -07:00 -
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>
Ahmed Ibrahim ·
2026-04-26 23:23:34 +00:00