core/tui: non-blocking MCP startup #6334

nornagon-openai · 2025-11-06T19:54:08Z

This makes MCP startup not block TUI startup. Messages sent while MCPs are booting will be queued.

Screen.Recording.2025-11-06.at.11.53.24.AM.mov

chatgpt-codex-connector

💡 Codex Review

Lines 332 to 336 in d54213e

    
           let initial_prompt_task_id = conversation 
        
               .submit(Op::UserTurn { 
        
                   items, 
        
                   cwd: default_cwd, 
        
                   approval_policy: default_approval_policy,

Queue CLI prompt until MCP startup completes

Session initialization now just spawns spawn_startup_job (see codex-rs/core/src/codex.rs lines 548‑617) so the McpConnectionManager remains empty until the background job finishes. The codex-exec CLI (and any other non‑TUI client that uses ConversationManager) still calls conversation.submit(Op::UserTurn { … }) immediately after SessionConfigured is received (lines 332‑336 below) and never waits for the new McpStartupUpdate/McpStartupComplete events. When that turn reaches run_turn (codex-rs/core/src/codex.rs line 1871) it reads mcp_connection_manager.list_all_tools().await, which is still empty, so the router exposes zero MCP tools. Because CLI invocations usually consist of a single turn, repo_map, file_context, etc. simply never run anymore. We now need to block or queue the first CLI submission until the MCP startup job finishes (or wait for McpStartupComplete) to avoid losing all MCP functionality outside the TUI.

ℹ️ 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/tui/src/chatwidget/tests.rs

nornagon-openai · 2025-11-06T20:38:31Z

@codex review

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".

chatgpt-codex-connector · 2025-11-06T20:47:23Z

codex-rs/core/src/codex.rs

+        // Start MCP servers in the background and stream startup updates.
+        match spawn_startup_job(
+            config.mcp_servers.clone(),
+            config.mcp_oauth_credentials_store_mode,
+            sess.services.mcp_connection_manager.clone(),


Prevent turns from starting before MCPs are ready

Session creation now instantiates an empty McpConnectionManager and immediately spawns spawn_startup_job (this hunk) instead of awaiting MCP clients. Nothing in core blocks submissions until that job finishes, yet other callers—e.g. the JSON‑RPC server’s SendUserTurn handler in codex-rs/app-server/src/codex_message_processor.rs and the RMCP integration tests—still call conversation.submit(Op::UserTurn { … }) as soon as the session is configured. run_turn snapshots the available tools at the top (codex-rs/core/src/codex.rs lines 1871‑1874), so if startup is still in flight those turns see an empty tool list and cannot invoke any MCP tools. This regresses every non‑TUI/CLI client: the first request after connecting silently runs without MCP integrations whenever startup takes a few seconds. We need to block/queue user turns until McpStartupComplete or gate run_turn so that MCP tools are always ready before executing the first task.

Useful? React with 👍 / 👎.

codex-rs/tui/src/chatwidget.rs

Co-authored-by: pakrym-oai <[email protected]>

codex-rs/core/src/mcp_connection_manager.rs

pakrym-oai · 2025-11-06T21:20:31Z

codex-rs/exec/src/lib.rs

+    });
    let mut error_seen = false;
    while let Some(event) = rx.recv().await {
+        if matches!(event.msg, EventMsg::McpStartupComplete(_))


Should this be a different/more generic event?

Feels almost like another SessionCOnfigured

pakrym-oai · 2025-11-06T21:22:13Z

codex-rs/protocol/src/protocol.rs

+    McpStartupUpdate(McpStartupUpdateEvent),
+
+    /// Aggregate MCP startup completion summary.
+    McpStartupComplete(McpStartupCompleteEvent),


It's pretty non-obvious that all consumers must wait for McpStartupComplete now before doing anything.

Can you add an integration test to ensure it always fires?

@pakrym-oai instead, i added code so that consumers don't need to wait on this :)

ultimately i'd like for session initialization to be refactored so it can be broken down into its various parts (e.g. it should be possible to navigate history before MCPs are booted), but I think that change escapes the scope of this PR.

pakrym-oai · 2025-11-12T19:03:21Z

codex-rs/core/src/codex.rs

+        if has_active_turn {
+            self.abort_all_tasks(TurnAbortReason::Interrupted).await;
+        } else {
+            self.cancel_mcp_startup().await;


why do we need to cancel mcp startup on control c? Do we ever cancel it today?

if an mcp takes a long time to start, it should be possible to interrupt it to let messages send.

This muddles interrupt op semantics. Interrup today is specific to a running a running turn. Now it can interrupt other things that happen in the background.

pakrym-oai · 2025-11-12T19:03:36Z

codex-rs/core/src/codex.rs

    cancellation_token: CancellationToken,
 ) -> CodexResult<TurnRunResult> {
-    let mcp_tools = sess.services.mcp_connection_manager.list_all_tools();
+    sess.wait_for_mcp_startup(cancellation_token.child_token())


can we encapsulate this wait in list_all_tools ?

pakrym-oai · 2025-11-12T19:04:19Z

codex-rs/core/src/codex.rs

+            }
+            loop {
+                tokio::select! {
+                    _ = cancellation_token.cancelled() => return Err(CodexErr::Interrupted),


consider or_cancel()

pakrym-oai · 2025-11-12T19:06:52Z

codex-rs/core/src/codex.rs

            .parse_tool_name(tool_name)
    }

+    async fn wait_for_mcp_startup(&self, cancellation_token: CancellationToken) -> CodexResult<()> {


I'd love to push more of this into connection manager and have some simpler interface that codex can use.

codex-rs/core/src/mcp_connection_manager.rs

pakrym-oai · 2025-11-12T19:09:23Z

codex-rs/core/src/mcp_connection_manager.rs

-    ///
-    /// The server name originates from the keys of the `mcp_servers` map in
-    /// the user configuration.
+struct McpConnectionManagerInner {


For other services we store the Arc directly in SessionServices. Why wrap here?

pakrym-oai · 2025-11-12T19:16:16Z

codex-rs/core/src/mcp_connection_manager.rs

+                Some(res) = join_set.join_next() => {
+                    match res {
+                        Ok((server_name, StartupOutcome::Ready { managed })) => {
+                            manager.add_client(server_name.clone(), managed).await;


what's the point of adding clients one by one? should we set all of them at once?

we don't create the ManagedClient until tools are listed, because ManagedClient includes the tool list.

pakrym-oai · 2025-11-12T19:17:16Z

codex-rs/core/src/mcp_connection_manager.rs

+    manager: McpConnectionManager,
+    tx_event: Sender<Event>,
+    auth_entries: HashMap<String, McpAuthStatusEntry>,
+) -> Result<McpStartupJobHandle> {


can this return the entire McpConnectionManagerInner struct? So we don't have to go through a ton of locks while initializing.

codex-rs/core/src/mcp_connection_manager.rs

pakrym-oai · 2025-11-12T19:18:09Z

codex-rs/core/src/mcp_connection_manager.rs

+                            manager.add_client(server_name.clone(), managed).await;
+                            summary.ready.push(server_name.clone());
+                            inflight.remove(&server_name);
+                            let _ = emit_update(


can we not ignore these errors?

codex-rs/core/src/mcp_connection_manager.rs

pakrym-oai · 2025-11-12T19:25:19Z

codex-rs/core/src/mcp_connection_manager.rs

+    server_name: String,
+    transport: McpServerTransportConfig,
+    store_mode: OAuthCredentialsStoreMode,
+    startup_timeout: Duration,


can this be handled via cancel_token ?

codex-rs/core/src/mcp_connection_manager.rs

pakrym-oai

Feels like cancellation and the startup in general can be greatly simplified in mcp_connection_manager.

Would be nice to push more logic around startup tracking from codex into mcp_connection_manager

nornagon-openai added 3 commits November 6, 2025 11:38

wip

7116501

better error formatting

9d3d4d8

wip

d54213e

nornagon-openai requested a review from gpeal November 6, 2025 19:54

chatgpt-codex-connector bot reviewed Nov 6, 2025

View reviewed changes

codex-rs/tui/src/chatwidget/tests.rs Outdated Show resolved Hide resolved

nornagon-openai added 3 commits November 6, 2025 12:09

fix tests

001321c

wait for mcps in codex exec

c1a7899

clippy

cad3186

chatgpt-codex-connector bot reviewed Nov 6, 2025

View reviewed changes

etraut-openai added the oai-pr PRs posted by Codex team members label Nov 6, 2025

pakrym-oai reviewed Nov 6, 2025

View reviewed changes

codex-rs/tui/src/chatwidget.rs Outdated Show resolved Hide resolved

Update codex-rs/tui/src/chatwidget.rs

8634bc4

Co-authored-by: pakrym-oai <[email protected]>

pakrym-oai reviewed Nov 6, 2025

View reviewed changes

codex-rs/core/src/mcp_connection_manager.rs Outdated Show resolved Hide resolved

pakrym-oai reviewed Nov 6, 2025

View reviewed changes

nornagon-openai added 3 commits November 10, 2025 15:09

run_turn blocks on mcp boot

94d4a35

Merge remote-tracking branch 'origin/main' into nornagon/mcp-startup

929486b

reuse existing test module

ea74843

gpeal mentioned this pull request Nov 11, 2025

codex-cli 0.55.0 opens in a zombie state #6317

Open

pakrym-oai reviewed Nov 12, 2025

View reviewed changes

codex-rs/core/src/mcp_connection_manager.rs Show resolved Hide resolved

pakrym-oai reviewed Nov 12, 2025

View reviewed changes

codex-rs/core/src/mcp_connection_manager.rs Show resolved Hide resolved

pakrym-oai reviewed Nov 12, 2025

View reviewed changes

codex-rs/core/src/mcp_connection_manager.rs Show resolved Hide resolved

pakrym-oai reviewed Nov 12, 2025

View reviewed changes

codex-rs/core/src/mcp_connection_manager.rs Show resolved Hide resolved

pakrym-oai reviewed Nov 12, 2025

View reviewed changes

codex-rs/core/src/mcp_connection_manager.rs Show resolved Hide resolved

pakrym-oai reviewed Nov 12, 2025

View reviewed changes

codex-rs/core/src/mcp_connection_manager.rs Show resolved Hide resolved

pakrym-oai reviewed Nov 12, 2025

View reviewed changes

	let initial_prompt_task_id = conversation
	.submit(Op::UserTurn {
	items,
	cwd: default_cwd,
	approval_policy: default_approval_policy,

core/tui: non-blocking MCP startup #6334

Are you sure you want to change the base?

core/tui: non-blocking MCP startup #6334

Conversation

nornagon-openai commented Nov 6, 2025 • edited by gpeal Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

nornagon-openai commented Nov 6, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pakrym-oai Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pakrym-oai Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

pakrym-oai left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nornagon-openai commented Nov 6, 2025 •

edited by gpeal

Loading

pakrym-oai Nov 12, 2025 •

edited

Loading

pakrym-oai Nov 12, 2025 •

edited

Loading