From a50cb205b731134f5978b777e442fdc2e2cfb65d Mon Sep 17 00:00:00 2001
From: Eric Traut <etraut@openai.com>
Date: Thu, 23 Apr 2026 14:08:40 -0700
Subject: [PATCH] Stabilize plugin MCP tools test (#19191)

## Summary

The plugin MCP tool-listing test could hide MCP startup failures by
polling `ListMcpTools` until its own 30s deadline. If the plugin MCP
server startup had already failed or timed out, the session-owned MCP
manager would keep returning an empty tool list, so CI only reported
`discovered tools: []` instead of the startup state that mattered.

This makes the test synchronize on `McpStartupComplete` for the sample
plugin MCP server before asserting listed tools, and gives the
Bazel-launched test server a larger startup window.

## Notes

Confidence is about 80%. The source path strongly supports the RCA: a
failed MCP startup is represented as an empty tool list through
`ListMcpTools`, so the old polling contract could not distinguish "not
ready yet" from "startup already failed." I could not retrieve the CI
execution-log artifact to confirm the exact hidden startup error, but
the observed Ubuntu Bazel failure matches this path: repeated
`ListMcpTools` responses with no tools until the test-local timeout
fired.

I think this is the right solution because it keeps plugin behavior
unchanged and fixes only the test contract. Future startup failures
should now report the `McpStartupComplete` failure/cancellation instead
of timing out on an empty tool snapshot.

This test was introduced in https://github.com/openai/codex/pull/12864.
---
 codex-rs/core/tests/suite/plugins.rs | 78 +++++++++++++++++++---------
 1 file changed, 54 insertions(+), 24 deletions(-)

diff --git a/codex-rs/core/tests/suite/plugins.rs b/codex-rs/core/tests/suite/plugins.rs
index 07602c1be..30aedccf0 100644
--- a/codex-rs/core/tests/suite/plugins.rs
+++ b/codex-rs/core/tests/suite/plugins.rs
@@ -6,6 +6,7 @@ use std::time::Duration;
 use std::time::Instant;
 
 use anyhow::Result;
+use anyhow::bail;
 use codex_features::Feature;
 use codex_login::CodexAuth;
 use codex_protocol::protocol::EventMsg;
@@ -72,7 +73,8 @@ fn write_plugin_mcp_plugin(home: &TempDir, command: &str) {
             r#"{{
   "mcpServers": {{
     "sample": {{
-      "command": "{command}"
+      "command": "{command}",
+      "startup_timeout_sec": 60.0
     }}
   }}
 }}"#
@@ -415,30 +417,58 @@ async fn plugin_mcp_tools_are_listed() -> Result<()> {
     write_plugin_mcp_plugin(codex_home.as_ref(), &rmcp_test_server_bin);
     let codex = build_plugin_test_codex(&server, codex_home).await?;
 
-    let tools_ready_deadline = Instant::now() + Duration::from_secs(30);
-    loop {
-        codex.submit(Op::ListMcpTools).await?;
-        let list_event = wait_for_event_with_timeout(
-            &codex,
-            |ev| matches!(ev, EventMsg::McpListToolsResponse(_)),
-            Duration::from_secs(10),
-        )
-        .await;
-        let EventMsg::McpListToolsResponse(tool_list) = list_event else {
-            unreachable!("event guard guarantees McpListToolsResponse");
-        };
-        if tool_list.tools.contains_key("mcp__sample__echo")
-            && tool_list.tools.contains_key("mcp__sample__image")
-        {
-            break;
-        }
-
-        let available_tools: Vec<&str> = tool_list.tools.keys().map(String::as_str).collect();
-        if Instant::now() >= tools_ready_deadline {
-            panic!("timed out waiting for plugin MCP tools; discovered tools: {available_tools:?}");
-        }
-        tokio::time::sleep(Duration::from_millis(200)).await;
+    let startup_event = wait_for_event_with_timeout(
+        &codex,
+        |ev| match ev {
+            EventMsg::McpStartupComplete(summary) => {
+                summary.ready.iter().any(|server| server == "sample")
+                    || summary
+                        .failed
+                        .iter()
+                        .any(|failure| failure.server == "sample")
+                    || summary.cancelled.iter().any(|server| server == "sample")
+            }
+            _ => false,
+        },
+        Duration::from_secs(70),
+    )
+    .await;
+    let EventMsg::McpStartupComplete(startup) = startup_event else {
+        unreachable!("event guard guarantees McpStartupComplete");
+    };
+    if let Some(failure) = startup
+        .failed
+        .iter()
+        .find(|failure| failure.server == "sample")
+    {
+        let error = &failure.error;
+        bail!("plugin MCP server failed to start: {error}");
     }
+    if startup.cancelled.iter().any(|server| server == "sample") {
+        bail!("plugin MCP server startup was cancelled");
+    }
+    assert!(
+        startup.ready.iter().any(|server| server == "sample"),
+        "expected plugin MCP server to be ready; startup summary: {startup:?}"
+    );
+
+    codex.submit(Op::ListMcpTools).await?;
+    let list_event = wait_for_event_with_timeout(
+        &codex,
+        |ev| matches!(ev, EventMsg::McpListToolsResponse(_)),
+        Duration::from_secs(10),
+    )
+    .await;
+    let EventMsg::McpListToolsResponse(tool_list) = list_event else {
+        unreachable!("event guard guarantees McpListToolsResponse");
+    };
+    let mut available_tools: Vec<&str> = tool_list.tools.keys().map(String::as_str).collect();
+    available_tools.sort_unstable();
+    assert!(
+        tool_list.tools.contains_key("mcp__sample__echo")
+            && tool_list.tools.contains_key("mcp__sample__image"),
+        "expected plugin MCP tools to be listed; discovered tools: {available_tools:?}"
+    );
 
     Ok(())
 }