From 88f87d907a91aea5e9ea38a3e9a653bfedd71f9b Mon Sep 17 00:00:00 2001 From: jif Date: Fri, 4 Sep 2026 13:06:37 +0000 Subject: [PATCH] Avoid port races in streamable HTTP tests (#42767) ## Why The test helper previously released its reserved port before starting the streamable HTTP server. Another concurrent test could claim that port and satisfy the readiness probe for the wrong process. ## What changed Start the child server on an OS-assigned port and wait for it to publish its actual bound address. Use that address to build the base URL returned to the streamable HTTP integration tests. GitOrigin-RevId: 606ae446f947d39424e4ec996ac647881777ff44 --- .../tests/streamable_http_test_support.rs | 55 ++++++++++--------- 1 file changed, 28 insertions(+), 27 deletions(-) diff --git a/codex-rs/rmcp-client/tests/streamable_http_test_support.rs b/codex-rs/rmcp-client/tests/streamable_http_test_support.rs index d289d6f83d..e7d6e2c52f 100644 --- a/codex-rs/rmcp-client/tests/streamable_http_test_support.rs +++ b/codex-rs/rmcp-client/tests/streamable_http_test_support.rs @@ -3,13 +3,15 @@ //! This support module starts the test HTTP server, launches a real //! `exec-server` when remote coverage is needed, and provides small helpers for //! creating RMCP clients and asserting round-trip behavior. +//! HTTP server startup uses the address published by the child that owns the listener. // This support module is included by multiple integration-test crates. Each // crate uses a different subset of the helpers, so dead-code warnings would // otherwise depend on which test file compiled the module. #![allow(dead_code)] -use std::net::TcpListener; +use std::net::SocketAddr; +use std::path::Path; use std::path::PathBuf; use std::process::Stdio; use std::sync::Arc; @@ -42,7 +44,6 @@ use serde_json::json; use tempfile::TempDir; use tokio::io::AsyncBufReadExt; use tokio::io::BufReader; -use tokio::net::TcpStream; use tokio::process::Child; use tokio::process::Command; use tokio::time::sleep; @@ -310,19 +311,23 @@ pub(crate) async fn arm_initialize_post_json_rpc_failure( } pub(crate) async fn spawn_streamable_http_server() -> anyhow::Result<(Child, String)> { - let listener = TcpListener::bind("127.0.0.1:0")?; - let port = listener.local_addr()?.port(); - drop(listener); - - let bind_addr = format!("127.0.0.1:{port}"); - let base_url = format!("http://{bind_addr}"); + let startup_dir = tempfile::tempdir()?; + let bound_addr_path = startup_dir.path().join("bound_addr"); + // Let the child reserve its own port; releasing a parent-owned port before + // spawning can let another test claim it and satisfy the readiness probe. let mut child = Command::new(streamable_http_server_bin()?) .kill_on_drop(true) - .env("MCP_STREAMABLE_HTTP_BIND_ADDR", &bind_addr) + .env("MCP_STREAMABLE_HTTP_BIND_ADDR", "127.0.0.1:0") + .env("MCP_STREAMABLE_HTTP_BOUND_ADDR_FILE", &bound_addr_path) .spawn()?; - wait_for_streamable_http_server(&mut child, &bind_addr, Duration::from_secs(5)).await?; - Ok((child, base_url)) + let address = wait_for_streamable_http_server( + &mut child, + &bound_addr_path, + Duration::from_secs(/*secs*/ 5), + ) + .await?; + Ok((child, format!("http://{address}"))) } /// Owns the exec-server process used by the remote-client integration test. @@ -394,9 +399,9 @@ async fn read_exec_server_listen_url(child: &mut Child) -> anyhow::Result anyhow::Result<()> { +) -> anyhow::Result { let deadline = Instant::now() + timeout; loop { @@ -406,26 +411,22 @@ async fn wait_for_streamable_http_server( )); } - let remaining = deadline.saturating_duration_since(Instant::now()); - if remaining.is_zero() { + if Instant::now() >= deadline { return Err(anyhow::anyhow!( - "timed out waiting for streamable HTTP server at {address}: deadline reached" + "timed out waiting for streamable HTTP server to publish its bound address" )); } - match tokio::time::timeout(remaining, TcpStream::connect(address)).await { - Ok(Ok(_)) => return Ok(()), - Ok(Err(error)) => { - if Instant::now() >= deadline { - return Err(anyhow::anyhow!( - "timed out waiting for streamable HTTP server at {address}: {error}" - )); + match std::fs::read_to_string(bound_addr_path) { + Ok(contents) => { + // The child creates the file before writing the address into it. + if let Ok(address) = contents.parse() { + return Ok(address); } } - Err(_) => { - return Err(anyhow::anyhow!( - "timed out waiting for streamable HTTP server at {address}: connect call timed out" - )); + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(error) => { + return Err(error).context("failed to read streamable HTTP server bound address"); } }