Merge remote-tracking branch 'origin/main' into linear/nap-244-cli-rewire-maestro-attachlistspawnkill-through-home-daemon

crates/maestro-cli/src/http.rs

@@ -3,6 +3,7 @@
 
 use std::path::PathBuf;
 use std::sync::{Arc, Mutex};
+use std::time::Duration;
 
 use reqwest::header::{HeaderMap, HeaderValue, COOKIE, SET_COOKIE};
 use reqwest::{Method, Response, StatusCode};
@@ -13,6 +14,30 @@ use crate::config::{CliConfig, ConfigError};
 
 const COOKIE_NAME: &str = "maestro_session";
 
+/// Time budget for establishing a TCP+TLS connection to the registry/daemon.
+const CONNECT_TIMEOUT: Duration = Duration::from_secs(5);
+/// Total time budget for a single request (connect + send + receive). Bounds
+/// the case where the peer is TCP-alive but never makes forward progress.
+const REQUEST_TIMEOUT: Duration = Duration::from_secs(30);
+
+/// Build a `reqwest::Client` with the CLI's standard timeouts and user-agent.
+///
+/// Centralizing construction here guarantees every client the CLI uses gets a
+/// connect *and* request timeout, so `maestro` never hangs forever on an
+/// unhealthy registry or daemon (NAP-262).
+pub fn build_client() -> reqwest::Client {
+    build_client_with_timeouts(CONNECT_TIMEOUT, REQUEST_TIMEOUT)
+}
+
+fn build_client_with_timeouts(connect: Duration, request: Duration) -> reqwest::Client {
+    reqwest::Client::builder()
+        .user_agent(concat!("maestro-cli/", env!("CARGO_PKG_VERSION")))
+        .connect_timeout(connect)
+        .timeout(request)
+        .build()
+        .expect("build reqwest client")
+}
+
 #[derive(Debug, thiserror::Error)]
 pub enum HttpError {
     #[error("http: {0}")]
@@ -72,10 +97,7 @@ pub struct RegistryClient {
 
 impl RegistryClient {
     pub fn new(base: String, cookie: Option<String>, config_path: PathBuf) -> Self {
-        let inner = reqwest::Client::builder()
-            .user_agent(concat!("maestro-cli/", env!("CARGO_PKG_VERSION")))
-            .build()
-            .expect("build reqwest client");
+        let inner = build_client();
         Self {
             base: base.trim_end_matches('/').to_string(),
             inner,
@@ -201,4 +223,33 @@ mod tests {
         let raw = "other=1; HttpOnly";
         assert_eq!(extract_cookie_value(raw, "maestro_session"), None);
     }
+
+    /// A peer that accepts the TCP connection but never sends a response must
+    /// not hang the client forever: the request timeout has to fire (NAP-262).
+    #[tokio::test]
+    async fn request_timeout_fires_when_peer_never_responds() {
+        use std::io::Read;
+        use std::net::TcpListener;
+
+        let listener = TcpListener::bind("127.0.0.1:0").unwrap();
+        let addr = listener.local_addr().unwrap();
+        // Accept one connection and sit on it without ever writing a response.
+        let handle = std::thread::spawn(move || {
+            if let Ok((mut sock, _)) = listener.accept() {
+                let mut buf = [0u8; 1024];
+                let _ = sock.read(&mut buf);
+                std::thread::sleep(Duration::from_secs(2));
+            }
+        });
+
+        let client = build_client_with_timeouts(Duration::from_secs(5), Duration::from_millis(300));
+        let err = client
+            .get(format!("http://{addr}/"))
+            .send()
+            .await
+            .expect_err("request should time out, not hang");
+        assert!(err.is_timeout(), "expected timeout error, got: {err}");
+
+        let _ = handle.join();
+    }
 }

crates/maestro-cli/src/policy.rs

@@ -36,7 +36,7 @@ pub struct PolicyView {
 
 pub async fn run_reload(config_path: &Path, out: &mut impl Write) -> Result<(), PolicyError> {
     let url = format!("{}/api/policy/reload", local_base_url(config_path)?);
-    let resp = reqwest::Client::new().post(&url).send().await?;
+    let resp = crate::http::build_client().post(&url).send().await?;
     let status = resp.status();
     if !status.is_success() {
         let body = resp.text().await.unwrap_or_default();
@@ -65,7 +65,7 @@ pub async fn run_show(
     out: &mut impl Write,
 ) -> Result<(), PolicyError> {
     let url = format!("{}/api/policy", local_base_url(config_path)?);
-    let resp = reqwest::Client::new().get(&url).send().await?;
+    let resp = crate::http::build_client().get(&url).send().await?;
     let status = resp.status();
     if !status.is_success() {
         let body = resp.text().await.unwrap_or_default();