From 865bd3a7732ff075ac59c57ee6829441c6d0d024 Mon Sep 17 00:00:00 2001 From: Eason Goodale Date: Fri, 8 Aug 2025 19:05:47 -0700 Subject: [PATCH] port discovery --- codex-rs/login/src/entrypoints.rs | 16 +---- codex-rs/login/src/server.rs | 24 +++++++- codex-rs/login/tests/headless.rs | 2 + codex-rs/login/tests/server.rs | 97 ++++++++++++++++++------------- 4 files changed, 81 insertions(+), 58 deletions(-) diff --git a/codex-rs/login/src/entrypoints.rs b/codex-rs/login/src/entrypoints.rs index 4e56c7e41b..ae17bc7651 100644 --- a/codex-rs/login/src/entrypoints.rs +++ b/codex-rs/login/src/entrypoints.rs @@ -1,5 +1,4 @@ -use std::io; -use std::net::TcpListener; + use std::path::Path; use std::process::Child; use std::process::Stdio; @@ -71,17 +70,6 @@ pub async fn login_with_chatgpt( .filter(|s| !s.is_empty()) .unwrap_or_else(|| crate::CLIENT_ID.to_string()); - match TcpListener::bind(("127.0.0.1", server::DEFAULT_PORT)) { - Ok(_sock) => { - // release immediately; server will bind next - } - Err(e) => { - if e.kind() == io::ErrorKind::AddrInUse { - return Err(io::Error::new(io::ErrorKind::AddrInUse, e)); - } - } - } - let codex_home = codex_home.to_path_buf(); let client_id_cloned = client_id.clone(); tokio::task::spawn_blocking(move || { @@ -95,6 +83,8 @@ pub async fn login_with_chatgpt( expose_state_endpoint: false, testing_timeout_secs: None, verbose, + #[cfg(feature = "http-e2e-tests")] + port_sender: None, }; server::run_local_login_server_with_options(opts) }) diff --git a/codex-rs/login/src/server.rs b/codex-rs/login/src/server.rs index f077a5df12..d1428421ae 100644 --- a/codex-rs/login/src/server.rs +++ b/codex-rs/login/src/server.rs @@ -3,6 +3,7 @@ use rand::RngCore; use reqwest::blocking::Client; use serde_json::json; use std::collections::HashMap; +use std::net::TcpListener; use std::path::Path; use std::path::PathBuf; #[cfg(feature = "http-e2e-tests")] @@ -36,6 +37,8 @@ pub struct LoginServerOptions { /// timeout after x secs for e2e tests pub testing_timeout_secs: Option, pub verbose: bool, + #[cfg(feature = "http-e2e-tests")] + pub port_sender: Option>, } // Only default issuer supported for platform/api bases @@ -58,13 +61,28 @@ pub fn run_local_login_server(codex_home: &Path, client_id: &str) -> std::io::Re expose_state_endpoint: false, testing_timeout_secs: None, verbose: false, + #[cfg(feature = "http-e2e-tests")] + port_sender: None, }; run_local_login_server_with_options(opts) } -pub fn run_local_login_server_with_options(opts: LoginServerOptions) -> std::io::Result<()> { - let addr = format!("127.0.0.1:{}", opts.port); - let server = Server::http(&addr).map_err(|e| std::io::Error::other(e.to_string()))?; +pub fn run_local_login_server_with_options(mut opts: LoginServerOptions) -> std::io::Result<()> { + let listener = TcpListener::bind(("127.0.0.1", opts.port)) + .map_err(|e| std::io::Error::other(e.to_string()))?; + let actual_port = listener + .local_addr() + .map_err(|e| std::io::Error::other(e.to_string()))? + .port(); + opts.port = actual_port; + + #[cfg(feature = "http-e2e-tests")] + if let Some(tx) = &opts.port_sender { + let _ = tx.send(actual_port); + } + + let server = Server::from_listener(listener, None) + .map_err(|e| std::io::Error::other(e.to_string()))?; let issuer = opts.issuer.clone(); let url_base = default_url_base(opts.port); diff --git a/codex-rs/login/tests/headless.rs b/codex-rs/login/tests/headless.rs index 6bcf12aa33..9ba92839c0 100644 --- a/codex-rs/login/tests/headless.rs +++ b/codex-rs/login/tests/headless.rs @@ -68,6 +68,8 @@ fn default_opts(tmp: &TempDir) -> LoginServerOptions { expose_state_endpoint: false, testing_timeout_secs: None, verbose: false, + #[cfg(feature = "http-e2e-tests")] + port_sender: None, } } diff --git a/codex-rs/login/tests/server.rs b/codex-rs/login/tests/server.rs index d9115c4da6..f705148807 100644 --- a/codex-rs/login/tests/server.rs +++ b/codex-rs/login/tests/server.rs @@ -9,17 +9,11 @@ use std::thread; use std::time::Duration; use tempfile::TempDir; -fn find_free_port() -> u16 { - TcpListener::bind("127.0.0.1:0") - .unwrap() - .local_addr() - .unwrap() - .port() -} - -fn start_mock_oauth_server(port: u16, behavior: MockBehavior) { +fn start_mock_oauth_server(behavior: MockBehavior) -> u16 { + let listener = TcpListener::bind(("127.0.0.1", 0)).unwrap(); + let port = listener.local_addr().unwrap().port(); thread::spawn(move || { - let server = tiny_http::Server::http(format!("127.0.0.1:{port}")).unwrap(); + let server = tiny_http::Server::from_listener(listener, None).unwrap(); for mut request in server.incoming_requests() { let url = request.url().to_string(); if request.method() == &tiny_http::Method::Post && url.starts_with("/oauth/token") { @@ -273,6 +267,7 @@ fn start_mock_oauth_server(port: u16, behavior: MockBehavior) { } } }); + port } #[derive(Clone, Copy)] @@ -315,27 +310,31 @@ fn http_get_follow_redirect(url: &str) -> (u16, String) { // 1) Happy path: writes auth.json and exits after /success #[test] fn login_server_happy_path() { - let oauth_port = find_free_port(); - start_mock_oauth_server(oauth_port, MockBehavior::Success); + let oauth_port = start_mock_oauth_server(MockBehavior::Success); let codex_home = TempDir::new().unwrap(); - let port = find_free_port(); + let (tx, rx) = std::sync::mpsc::channel(); let issuer = format!("http://127.0.0.1:{oauth_port}"); let opts = LoginServerOptions { codex_home: codex_home.path().to_path_buf(), client_id: "test-client".to_string(), issuer: issuer.clone(), - port, + port: 0, open_browser: false, redeem_credits: true, expose_state_endpoint: true, testing_timeout_secs: Some(5), verbose: false, + #[cfg(feature = "http-e2e-tests")] + port_sender: Some(tx), }; let handle = thread::spawn(move || run_local_login_server_with_options(opts).unwrap()); + // Receive the bound port from the server + let port = rx.recv().unwrap(); + // Wait for server to bind wait_for_state_endpoint(port, Duration::from_secs(5)); @@ -372,26 +371,28 @@ fn login_server_happy_path() { // 1b) needs_setup=true when onboarding incomplete and is_org_owner=true #[test] fn login_server_needs_setup_true_and_params_present() { - let oauth_port = find_free_port(); - start_mock_oauth_server(oauth_port, MockBehavior::SuccessNeedsSetup); + let oauth_port = start_mock_oauth_server(MockBehavior::SuccessNeedsSetup); let codex_home = TempDir::new().unwrap(); - let port = find_free_port(); + let (tx, rx) = std::sync::mpsc::channel(); let issuer = format!("http://127.0.0.1:{oauth_port}"); let opts = LoginServerOptions { codex_home: codex_home.path().to_path_buf(), client_id: "test-client".to_string(), issuer: issuer.clone(), - port, + port: 0, open_browser: false, redeem_credits: true, expose_state_endpoint: true, testing_timeout_secs: Some(5), verbose: false, + #[cfg(feature = "http-e2e-tests")] + port_sender: Some(tx), }; let handle = thread::spawn(move || run_local_login_server_with_options(opts).unwrap()); + let port = rx.recv().unwrap(); wait_for_state_endpoint(port, Duration::from_secs(5)); let state_url = format!("http://127.0.0.1:{port}/__test/state"); let (_s, state, _) = http_get(&state_url); @@ -411,26 +412,28 @@ fn login_server_needs_setup_true_and_params_present() { // 1c) org/project from ID token only should appear in redirect (fallback logic) #[test] fn login_server_id_token_fallback_for_org_and_project() { - let oauth_port = find_free_port(); - start_mock_oauth_server(oauth_port, MockBehavior::SuccessIdClaimsOrgProject); + let oauth_port = start_mock_oauth_server(MockBehavior::SuccessIdClaimsOrgProject); let codex_home = TempDir::new().unwrap(); - let port = find_free_port(); + let (tx, rx) = std::sync::mpsc::channel(); let issuer = format!("http://127.0.0.1:{oauth_port}"); let opts = LoginServerOptions { codex_home: codex_home.path().to_path_buf(), client_id: "test-client".to_string(), issuer: issuer.clone(), - port, + port: 0, open_browser: false, redeem_credits: true, expose_state_endpoint: true, testing_timeout_secs: Some(5), verbose: false, + #[cfg(feature = "http-e2e-tests")] + port_sender: Some(tx), }; let handle = thread::spawn(move || run_local_login_server_with_options(opts).unwrap()); + let port = rx.recv().unwrap(); wait_for_state_endpoint(port, Duration::from_secs(5)); let state_url = format!("http://127.0.0.1:{port}/__test/state"); let (_s, state, _) = http_get(&state_url); @@ -447,26 +450,28 @@ fn login_server_id_token_fallback_for_org_and_project() { // 1d) Missing org/project in claims -> skip token-exchange, persist tokens without API key, still success #[test] fn login_server_skips_exchange_when_no_org_or_project() { - let oauth_port = find_free_port(); - start_mock_oauth_server(oauth_port, MockBehavior::MissingOrgSkipExchange); + let oauth_port = start_mock_oauth_server(MockBehavior::MissingOrgSkipExchange); let codex_home = TempDir::new().unwrap(); - let port = find_free_port(); + let (tx, rx) = std::sync::mpsc::channel(); let issuer = format!("http://127.0.0.1:{oauth_port}"); let opts = LoginServerOptions { codex_home: codex_home.path().to_path_buf(), client_id: "test-client".to_string(), issuer: issuer.clone(), - port, + port: 0, open_browser: false, redeem_credits: true, expose_state_endpoint: true, testing_timeout_secs: Some(5), verbose: false, + #[cfg(feature = "http-e2e-tests")] + port_sender: Some(tx), }; let handle = thread::spawn(move || run_local_login_server_with_options(opts).unwrap()); + let port = rx.recv().unwrap(); wait_for_state_endpoint(port, Duration::from_secs(5)); let state_url = format!("http://127.0.0.1:{port}/__test/state"); let (_s, state, _) = http_get(&state_url); @@ -491,9 +496,8 @@ fn login_server_skips_exchange_when_no_org_or_project() { // 2) State mismatch returns 400 and server stays up #[test] fn login_server_state_mismatch() { - let oauth_port = find_free_port(); - start_mock_oauth_server(oauth_port, MockBehavior::Success); - let port = find_free_port(); + let oauth_port = start_mock_oauth_server(MockBehavior::Success); + let (tx, rx) = std::sync::mpsc::channel(); let codex_home = TempDir::new().unwrap(); let issuer = format!("http://127.0.0.1:{oauth_port}"); @@ -501,14 +505,17 @@ fn login_server_state_mismatch() { codex_home: codex_home.path().into(), client_id: "test-client".into(), issuer, - port, + port: 0, open_browser: false, redeem_credits: false, expose_state_endpoint: true, testing_timeout_secs: Some(5), verbose: false, + #[cfg(feature = "http-e2e-tests")] + port_sender: Some(tx), }; let handle = thread::spawn(move || run_local_login_server_with_options(opts).unwrap()); + let port = rx.recv().unwrap(); wait_for_state_endpoint(port, Duration::from_secs(5)); let cb_url = format!("http://127.0.0.1:{port}/auth/callback?code=abc&state=wrong"); @@ -524,23 +531,25 @@ fn login_server_state_mismatch() { // 3) Missing code returns 400 #[test] fn login_server_missing_code() { - let oauth_port = find_free_port(); - start_mock_oauth_server(oauth_port, MockBehavior::Success); - let port = find_free_port(); + let oauth_port = start_mock_oauth_server(MockBehavior::Success); + let (tx, rx) = std::sync::mpsc::channel(); let codex_home = TempDir::new().unwrap(); let issuer = format!("http://127.0.0.1:{oauth_port}"); let opts = LoginServerOptions { codex_home: codex_home.path().into(), client_id: "test-client".into(), issuer, - port, + port: 0, open_browser: false, redeem_credits: false, expose_state_endpoint: true, testing_timeout_secs: Some(5), verbose: false, + #[cfg(feature = "http-e2e-tests")] + port_sender: Some(tx), }; let handle = thread::spawn(move || run_local_login_server_with_options(opts).unwrap()); + let port = rx.recv().unwrap(); wait_for_state_endpoint(port, Duration::from_secs(5)); // Fetch state @@ -560,23 +569,25 @@ fn login_server_missing_code() { // 4) Token endpoint error returns 500 (on code exchange) and server stays up #[test] fn login_server_token_exchange_error() { - let oauth_port = find_free_port(); - start_mock_oauth_server(oauth_port, MockBehavior::TokenError); - let port = find_free_port(); + let oauth_port = start_mock_oauth_server(MockBehavior::TokenError); + let (tx, rx) = std::sync::mpsc::channel(); let codex_home = TempDir::new().unwrap(); let issuer = format!("http://127.0.0.1:{oauth_port}"); let opts = LoginServerOptions { codex_home: codex_home.path().into(), client_id: "test-client".into(), issuer, - port, + port: 0, open_browser: false, redeem_credits: false, expose_state_endpoint: true, testing_timeout_secs: Some(5), verbose: false, + #[cfg(feature = "http-e2e-tests")] + port_sender: Some(tx), }; let handle = thread::spawn(move || run_local_login_server_with_options(opts).unwrap()); + let port = rx.recv().unwrap(); wait_for_state_endpoint(port, Duration::from_secs(5)); let state = ureq::get(&format!("http://127.0.0.1:{port}/__test/state")) .call() @@ -594,23 +605,25 @@ fn login_server_token_exchange_error() { #[test] fn login_server_credit_redemption_best_effort() { // Mock behavior success for token endpoints, but have redeem endpoint return 500 by not matching path (using different port) - let oauth_port = find_free_port(); - start_mock_oauth_server(oauth_port, MockBehavior::Success); - let port = find_free_port(); + let oauth_port = start_mock_oauth_server(MockBehavior::Success); + let (tx, rx) = std::sync::mpsc::channel(); let codex_home = TempDir::new().unwrap(); let issuer = format!("http://127.0.0.1:{oauth_port}"); let opts = LoginServerOptions { codex_home: codex_home.path().into(), client_id: "test-client".into(), issuer, - port, + port: 0, open_browser: false, redeem_credits: true, expose_state_endpoint: true, testing_timeout_secs: Some(5), verbose: false, + #[cfg(feature = "http-e2e-tests")] + port_sender: Some(tx), }; let handle = thread::spawn(move || run_local_login_server_with_options(opts).unwrap()); + let port = rx.recv().unwrap(); wait_for_state_endpoint(port, Duration::from_secs(5)); let state = ureq::get(&format!("http://127.0.0.1:{port}/__test/state")) .call()