From 6d2168f06ae275d5e1f73cabf935d2bcc8549998 Mon Sep 17 00:00:00 2001 From: jif Date: Fri, 26 Jun 2026 09:27:41 +0100 Subject: [PATCH] Reuse MCP runtimes when selected availability changes nothing (#30148) ## Why MCP runtime reuse was keyed by every ready selected-capability environment, even when an environment contributed no MCP servers or connectors. For example: 1. a global stdio MCP is running; 2. a selected remote environment contains only a skill; 3. that environment becomes ready; 4. the MCP and connector projection stays exactly the same; 5. Codex nevertheless rebuilds the MCP manager and restarts the global stdio process. That restart can interrupt active calls and discard process-local state even though nothing about MCP changed. ## What changes When selected-environment availability changes, Codex now resolves the candidate MCP and connector projection before deciding whether to replace the runtime: - if the winning MCP servers or their ownership change, rebuild as before; - if the selected connector snapshot changes, rebuild as before; - if an enabled MCP is explicitly bound to an environment whose availability changed, rebuild as before; - otherwise, keep the exact live manager and processes, and update only the availability input remembered by the snapshot. ```text ready selected environments: [] -> [skills-env] resolved MCP servers: {global_probe} -> {global_probe} resolved connectors: {} -> {} result: reuse manager; keep the same process ``` The comparison uses the resolved winning servers and their sources, so plugin/config ownership remains part of the runtime identity. ## Existing stack coverage The integration PR directly below this one already covers both rebuild boundaries: a selected MCP becomes callable and a selected connector tool becomes model-visible when their environment becomes available. It also verifies that an unchanged selected MCP runtime keeps its process. This PR does not add another remote-attachment integration scenario for the no-change optimization. `environment/add` returns before readiness, and app-server does not currently expose a deterministic readiness signal for an environment that contributes only skills. Keeping a fixed-delay test would add flake risk; adding a new readiness API would be outside this fix. ## Scope and assumptions - This does not change skill discovery, World State rendering, or plugin metadata caching. - This does not add file watching or hot reload behavior. - This does not change disconnect/reconnect handling. - Selected environment IDs and their capability contents retain the stack's existing stability assumption. - Delayed `required = true` executor MCP behavior remains out of scope. --- codex-rs/codex-mcp/src/catalog.rs | 5 +++++ codex-rs/core/src/session/mcp.rs | 30 ++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/codex-rs/codex-mcp/src/catalog.rs b/codex-rs/codex-mcp/src/catalog.rs index 689f93d81..d60ea843d 100644 --- a/codex-rs/codex-mcp/src/catalog.rs +++ b/codex-rs/codex-mcp/src/catalog.rs @@ -371,6 +371,11 @@ impl ResolvedMcpCatalog { .collect() } + /// Returns whether both catalogs resolve to the same winning servers and sources. + pub fn has_same_servers(&self, other: &Self) -> bool { + self.servers == other.servers + } + /// Replaces the resolved server set while preserving known server sources. /// /// Names not present in the existing catalog are treated as config-owned. diff --git a/codex-rs/core/src/session/mcp.rs b/codex-rs/core/src/session/mcp.rs index 83a59c960..25d95e30e 100644 --- a/codex-rs/core/src/session/mcp.rs +++ b/codex-rs/core/src/session/mcp.rs @@ -133,6 +133,36 @@ impl Session { &available_environment_ids, ) .await; + let changed_environment_is_used_by_mcp = mcp_config + .mcp_server_catalog + .configured_servers() + .values() + .any(|server| { + let was_available = current + .available_environment_ids() + .contains(&server.environment_id); + let is_available = available_environment_ids.contains(&server.environment_id); + server.enabled && was_available != is_available + }); + if !changed_environment_is_used_by_mcp + && current + .config() + .mcp_server_catalog + .has_same_servers(&mcp_config.mcp_server_catalog) + && current.config().connector_snapshot == mcp_config.connector_snapshot + { + // Availability is only an input to the MCP projection. When that input changes but + // the projected servers and connectors do not, advance the input key without + // replacing the live manager and restarting its processes. + let runtime = Arc::new(McpRuntimeSnapshot::new( + Arc::new(current.config().clone()), + current.manager_arc(), + current.runtime_context().clone(), + available_environment_ids, + )); + self.services.mcp_runtime.store(Some(Arc::clone(&runtime))); + return runtime; + } self.refresh_mcp_servers_inner( turn_context, mcp_config,