test(cli): wait for the server's own listening line and retry on a port race

crates/tole-cli/tests/common/mod.rs

@@ -0,0 +1,58 @@
+//! Shared helpers for tests that spawn `tole serve`.
+//!
+//! Picking a port by binding `127.0.0.1:0` and closing it is a race: before
+//! the server binds it, the same ephemeral port can be taken by another
+//! test's client socket, the server exits with "Address already in use",
+//! and a health poll then times out (or, worse, reaches a different
+//! listener). Readiness is therefore taken from the server's own
+//! "listening on" line, and a server that exits instead is retried on a
+//! fresh port.
+
+#![allow(dead_code)]
+
+use std::io::{BufRead, BufReader};
+use std::net::TcpListener;
+use std::process::{Child, Command, Stdio};
+use std::sync::mpsc;
+use std::time::Duration;
+
+const ATTEMPTS: usize = 5;
+const READY_TIMEOUT: Duration = Duration::from_secs(20);
+
+fn candidate_port() -> u16 {
+    TcpListener::bind("127.0.0.1:0")
+        .unwrap()
+        .local_addr()
+        .unwrap()
+        .port()
+}
+
+/// Spawn the command `make(port)` builds until one instance reports
+/// "listening on". Its stderr is forwarded to this test's stderr (so it
+/// shows up in a failure report). Returns the running child and its port.
+pub fn spawn_listening(mut make: impl FnMut(u16) -> Command) -> (Child, u16) {
+    for _ in 0..ATTEMPTS {
+        let port = candidate_port();
+        let mut cmd = make(port);
+        cmd.stderr(Stdio::piped());
+        let mut child = cmd.spawn().expect("spawn tole");
+        let stderr = child.stderr.take().expect("stderr piped");
+        let (tx, rx) = mpsc::channel();
+        std::thread::spawn(move || {
+            for line in BufReader::new(stderr).lines().map_while(Result::ok) {
+                if line.contains("listening on") {
+                    let _ = tx.send(true);
+                }
+                eprintln!("{line}");
+            }
+            // EOF: the process exited (or closed stderr) before listening.
+            let _ = tx.send(false);
+        });
+        if rx.recv_timeout(READY_TIMEOUT) == Ok(true) {
+            return (child, port);
+        }
+        let _ = child.kill();
+        let _ = child.wait();
+    }
+    panic!("tole serve did not start listening after {ATTEMPTS} attempts");
+}

crates/tole-cli/tests/mcp_http_hardening.rs

@@ -6,10 +6,12 @@
 #![cfg(all(feature = "shell-tools", feature = "mcp-http"))]
 
 use std::io::{Read, Write};
-use std::net::{TcpListener, TcpStream};
+use std::net::TcpStream;
 use std::process::{Child, Command, Stdio};
 use std::time::Duration;
 
+mod common;
+
 const TOKEN: &str = "t0ken";
 const INIT: &str = r#"{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"t","version":"1"}}}"#;
 
@@ -20,13 +22,9 @@ struct Server {
 
 impl Server {
     fn spawn() -> Self {
-        let port = TcpListener::bind("127.0.0.1:0")
-            .unwrap()
-            .local_addr()
-            .unwrap()
-            .port();
-        let child = Command::new(env!("CARGO_BIN_EXE_tole"))
-            .args([
+        let (child, port) = common::spawn_listening(|port| {
+            let mut cmd = Command::new(env!("CARGO_BIN_EXE_tole"));
+            cmd.args([
                 "serve",
                 "--transport",
                 "mcp",
@@ -36,20 +34,10 @@ impl Server {
                 TOKEN,
             ])
             .stdin(Stdio::null())
-            .stdout(Stdio::null())
-            .stderr(Stdio::null())
-            .spawn()
-            .expect("spawn tole serve --transport mcp");
-        let me = Self { child, port };
-        // Ready = it answers TCP. (A wrong-token probe would burn the
-        // limiter budget the tests below measure.)
-        for _ in 0..100 {
-            if TcpStream::connect(("127.0.0.1", port)).is_ok() {
-                return me;
-            }
-            std::thread::sleep(Duration::from_millis(100));
-        }
-        panic!("mcp server never listened");
+            .stdout(Stdio::null());
+            cmd
+        });
+        Self { child, port }
     }
 
     fn connect(&self) -> TcpStream {

crates/tole-cli/tests/serve_approvals.rs

@@ -15,31 +15,28 @@ use std::sync::atomic::{AtomicUsize, Ordering};
 use std::sync::Arc;
 use std::time::Duration;
 
+mod common;
+
 struct ServeProcess {
     child: Child,
     port: u16,
 }
 
 impl ServeProcess {
-    fn spawn(env: &[(&str, &str)], token: &str, port: u16) -> Self {
-        let mut cmd = Command::new(env!("CARGO_BIN_EXE_tole"));
-        cmd.args(["serve", "--port", &port.to_string(), "--token", token])
-            .stdin(Stdio::null())
-            .stdout(Stdio::null())
-            .stderr(Stdio::inherit());
-        for (k, v) in env {
-            cmd.env(k, v);
-        }
-        let child = cmd.spawn().expect("spawn tole serve");
-        let me = Self { child, port };
-        // Health-poll until the listener is up.
-        for _ in 0..50 {
-            if me.http("GET", "/health", None).0 == 200 {
-                return me;
+    /// `_port` is ignored: a free port is chosen (and retried) by
+    /// `common::spawn_listening`, which also waits for the listener.
+    fn spawn(env: &[(&str, &str)], token: &str, _port: u16) -> Self {
+        let (child, port) = common::spawn_listening(|port| {
+            let mut cmd = Command::new(env!("CARGO_BIN_EXE_tole"));
+            cmd.args(["serve", "--port", &port.to_string(), "--token", token])
+                .stdin(Stdio::null())
+                .stdout(Stdio::null());
+            for (k, v) in env {
+                cmd.env(k, v);
             }
-            std::thread::sleep(Duration::from_millis(100));
-        }
-        panic!("serve never became healthy");
+            cmd
+        });
+        Self { child, port }
     }
 
     fn http(&self, method: &str, path: &str, body: Option<&str>) -> (u16, Value) {

crates/tole-cli/tests/serve_http_hardening.rs

@@ -8,39 +8,27 @@
 #![cfg(feature = "shell-tools")]
 
 use std::io::{Read, Write};
-use std::net::{TcpListener, TcpStream};
+use std::net::TcpStream;
 use std::process::{Child, Command, Stdio};
 use std::time::{Duration, Instant};
 
+mod common;
+
 struct Serve {
     child: Child,
     port: u16,
 }
 
 impl Serve {
     fn spawn() -> Self {
-        let port = TcpListener::bind("127.0.0.1:0")
-            .unwrap()
-            .local_addr()
-            .unwrap()
-            .port();
-        let child = Command::new(env!("CARGO_BIN_EXE_tole"))
-            .args(["serve", "--port", &port.to_string(), "--token", "t0ken"])
-            .stdin(Stdio::null())
-            .stdout(Stdio::null())
-            .stderr(Stdio::inherit())
-            .spawn()
-            .expect("spawn tole serve");
-        let me = Self { child, port };
-        for _ in 0..50 {
-            if let Ok(r) = me.raw("GET /health HTTP/1.1\r\nConnection: close\r\n\r\n") {
-                if r.starts_with("HTTP/1.1 200") {
-                    return me;
-                }
-            }
-            std::thread::sleep(Duration::from_millis(100));
-        }
-        panic!("serve never became healthy");
+        let (child, port) = common::spawn_listening(|port| {
+            let mut cmd = Command::new(env!("CARGO_BIN_EXE_tole"));
+            cmd.args(["serve", "--port", &port.to_string(), "--token", "t0ken"])
+                .stdin(Stdio::null())
+                .stdout(Stdio::null());
+            cmd
+        });
+        Self { child, port }
     }
 
     fn connect(&self) -> std::io::Result<TcpStream> {
@@ -166,3 +154,33 @@ fn request_line_and_headers_share_one_size_ceiling() {
         status(&r)
     );
 }
+
+/// The port-race guard itself: an instance that exits before listening
+/// (as `tole serve` does on "Address already in use") is retried on a new
+/// port, and readiness comes from the "listening on" line.
+#[cfg(unix)]
+#[test]
+fn spawn_listening_retries_an_instance_that_exits_before_listening() {
+    let mut attempts = 0;
+    let (mut child, port) = common::spawn_listening(|port| {
+        attempts += 1;
+        let script = if attempts == 1 {
+            "echo 'binding: Address already in use' >&2; exit 1".to_string()
+        } else {
+            format!("echo 'listening on http://127.0.0.1:{port}' >&2; sleep 30")
+        };
+        let mut cmd = Command::new("sh");
+        cmd.args(["-c", &script])
+            .stdin(Stdio::null())
+            .stdout(Stdio::null());
+        cmd
+    });
+    assert_eq!(attempts, 2, "the first instance exited and was retried");
+    assert!(port > 0);
+    assert!(
+        child.try_wait().unwrap().is_none(),
+        "the second one is running"
+    );
+    let _ = child.kill();
+    let _ = child.wait();
+}

crates/tole-cli/tests/serve_session_cancel.rs

@@ -16,6 +16,8 @@ use std::process::{Child, Command, Stdio};
 use std::sync::Arc;
 use std::time::Duration;
 
+mod common;
+
 struct ServeProcess {
     child: Child,
     addr: String,
@@ -24,9 +26,6 @@ struct ServeProcess {
 
 impl ServeProcess {
     fn spawn(base_url: &str) -> Arc<Self> {
-        let port_listener = TcpListener::bind("127.0.0.1:0").unwrap();
-        let port = port_listener.local_addr().unwrap().port();
-        drop(port_listener);
         let cwd = std::env::temp_dir().join(format!(
             "tole-serve-cancel-{}-{:x}",
             std::time::SystemTime::now()
@@ -36,31 +35,33 @@ impl ServeProcess {
             std::process::id()
         ));
         std::fs::create_dir_all(&cwd).unwrap();
-        let mut cmd = Command::new(env!("CARGO_BIN_EXE_tole"));
-        cmd.args([
-            "serve",
-            "--port",
-            &port.to_string(),
-            "--workspace",
-            cwd.to_string_lossy().as_ref(),
-        ])
-        .stdin(Stdio::null())
-        .stdout(Stdio::null())
-        .stderr(Stdio::null())
-        .env_remove("TOLE_MEMORY")
-        .env_remove("TOLE_MEMORY_NAMESPACE")
-        .env_remove("TOLE_TRUST");
-        cmd.env("TOLE_BASE_URL", base_url);
-        cmd.env("TOLE_MODEL", "mock-model");
-        cmd.env("TOLE_API_KEY", "sk-test");
         let token = "test-token-123".to_string();
-        cmd.env("TOLE_SERVE_TOKEN", &token);
-        let child = cmd.spawn().expect("spawn tole serve");
+        let (child, port) = common::spawn_listening(|port| {
+            let mut cmd = Command::new(env!("CARGO_BIN_EXE_tole"));
+            cmd.args([
+                "serve",
+                "--port",
+                &port.to_string(),
+                "--workspace",
+                cwd.to_string_lossy().as_ref(),
+            ])
+            .stdin(Stdio::null())
+            .stdout(Stdio::null())
+            .env_remove("TOLE_MEMORY")
+            .env_remove("TOLE_MEMORY_NAMESPACE")
+            .env_remove("TOLE_TRUST");
+            cmd.env("TOLE_BASE_URL", base_url);
+            cmd.env("TOLE_MODEL", "mock-model");
+            cmd.env("TOLE_API_KEY", "sk-test");
+            cmd.env("TOLE_SERVE_TOKEN", &token);
+            cmd
+        });
         let proc = Arc::new(Self {
             child,
             addr: format!("127.0.0.1:{port}"),
             token,
         });
+        // The listener is up; also confirm the HTTP face answers.
         let deadline = std::time::Instant::now() + Duration::from_secs(20);
         while std::time::Instant::now() < deadline {
             if let Some((200, _)) = proc.request("GET", "/health", "") {