From 34138bbc3843afb8a89321db0acae797e352b04e Mon Sep 17 00:00:00 2001 From: Ogulcan Celik Date: Tue, 21 Apr 2026 19:11:22 +0300 Subject: [PATCH] fix: restore headless test reliability on macos --- src/detect.rs | 58 +++++++++++++++++++++++++++++++++------- src/server/autodetect.rs | 49 ++++++++++++++++----------------- src/server/headless.rs | 44 +++++++++++++++++++++++++----- src/update.rs | 4 +-- tests/multi_client.rs | 8 +++++- tests/support/mod.rs | 8 +++++- 6 files changed, 127 insertions(+), 44 deletions(-) diff --git a/src/detect.rs b/src/detect.rs index 55a7b2a5..4a0b8e7a 100644 --- a/src/detect.rs +++ b/src/detect.rs @@ -582,21 +582,38 @@ pub fn foreground_job(child_pid: u32) -> Option fn normalized_process_name(process: &crate::platform::ForegroundProcess) -> String { let effective = process.argv0.as_deref().unwrap_or(&process.name); let lower_effective = effective.to_lowercase(); - let lower_cmdline = process - .cmdline - .as_deref() - .unwrap_or_default() - .to_lowercase(); - if lower_effective == "node" - && (lower_cmdline.contains("/codex") || lower_cmdline.contains("@openai/codex")) - { - return "codex".to_string(); + if is_generic_runtime_or_shell(&lower_effective) { + if let Some(wrapped_agent) = + wrapped_agent_name_from_cmdline(process.cmdline.as_deref().unwrap_or_default()) + { + return wrapped_agent; + } } effective.to_string() } +fn wrapped_agent_name_from_cmdline(cmdline: &str) -> Option { + for token in cmdline.split_whitespace() { + let trimmed = token.trim_matches(|c| matches!(c, '"' | '\'')); + if trimmed.is_empty() || trimmed.starts_with('-') { + continue; + } + + let basename = std::path::Path::new(trimmed) + .file_name() + .and_then(|name| name.to_str()) + .unwrap_or(trimmed); + let Some(agent) = parse_agent_label(basename) else { + continue; + }; + return Some(agent_label(agent).to_string()); + } + + None +} + fn process_priority(process: &crate::platform::ForegroundProcess, normalized_name: &str) -> u8 { let lower_name = normalized_name.to_lowercase(); if lower_name != process.name.to_lowercase() { @@ -699,6 +716,29 @@ mod tests { ); } + #[test] + fn identify_agent_in_job_detects_shell_wrapped_pi() { + let job = crate::platform::ForegroundJob { + process_group_id: 123, + processes: vec![crate::platform::ForegroundProcess { + pid: 1, + name: "sh".to_string(), + argv0: None, + cmdline: Some("/bin/sh /tmp/test-bin/pi".to_string()), + }], + }; + + assert_eq!( + identify_agent_in_job(&job), + Some((Agent::Pi, "pi".to_string())) + ); + } + + #[test] + fn wrapped_agent_name_from_cmdline_ignores_plain_shell_flags() { + assert_eq!(wrapped_agent_name_from_cmdline("bash -lc"), None); + } + // ---- Workspace state rollup ---- // ---- No agent → Unknown ---- diff --git a/src/server/autodetect.rs b/src/server/autodetect.rs index b0ca2a2e..c4a07a2d 100644 --- a/src/server/autodetect.rs +++ b/src/server/autodetect.rs @@ -189,40 +189,37 @@ mod tests { use super::*; use std::os::unix::net::UnixListener; - fn temp_socket_path(name: &str) -> std::path::PathBuf { - let unique = format!( - "herdr-autodetect-test-{}-{}-{}", - name, - std::process::id(), - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_nanos() - ); - std::env::temp_dir().join(unique).join("test.sock") + fn unique_test_dir(name: &str) -> std::path::PathBuf { + let nanos = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos(); + std::path::PathBuf::from(format!("/tmp/ha-{name}-{}-{nanos}", std::process::id())) } #[test] fn is_server_listening_returns_false_for_nonexistent_path() { - let path = temp_socket_path("nonexistent"); + let dir = unique_test_dir("nonexistent"); + let path = dir.join("s.sock"); assert!(!is_server_listening_at(&path)); } #[test] fn is_server_listening_returns_true_for_live_socket() { - let dir = temp_socket_path("live"); + let dir = unique_test_dir("live"); std::fs::create_dir_all(&dir).unwrap(); - let path = dir.join("test.sock"); + let path = dir.join("s.sock"); let _listener = UnixListener::bind(&path).unwrap(); assert!(is_server_listening_at(&path)); + let _ = std::fs::remove_dir_all(dir); } #[test] fn is_server_listening_returns_false_for_stale_socket() { - let dir = temp_socket_path("stale"); + let dir = unique_test_dir("stale"); std::fs::create_dir_all(&dir).unwrap(); - let path = dir.join("test.sock"); + let path = dir.join("s.sock"); // Create a socket and immediately drop the listener. // This leaves a stale socket file with nobody listening. @@ -232,13 +229,14 @@ mod tests { // The socket file exists but nobody is listening. assert!(!is_server_listening_at(&path)); + let _ = std::fs::remove_dir_all(dir); } #[test] fn is_server_listening_returns_false_when_listener_dropped() { - let dir = temp_socket_path("dropped"); + let dir = unique_test_dir("dropped"); std::fs::create_dir_all(&dir).unwrap(); - let path = dir.join("test.sock"); + let path = dir.join("s.sock"); // Bind and immediately drop the listener. drop(UnixListener::bind(&path).unwrap()); @@ -251,34 +249,36 @@ mod tests { #[test] fn wait_for_server_socket_succeeds_immediately() { - let dir = temp_socket_path("wait-ok"); + let dir = unique_test_dir("wait-ok"); std::fs::create_dir_all(&dir).unwrap(); - let path = dir.join("test.sock"); + let path = dir.join("s.sock"); let _listener = UnixListener::bind(&path).unwrap(); // Should succeed immediately (socket is already ready). let result = wait_for_server_socket(&path, Duration::from_millis(100)); assert!(result.is_ok()); + let _ = std::fs::remove_dir_all(dir); } #[test] fn wait_for_server_socket_times_out() { - let dir = temp_socket_path("wait-timeout"); + let dir = unique_test_dir("wait-timeout"); std::fs::create_dir_all(&dir).unwrap(); - let path = dir.join("test.sock"); + let path = dir.join("s.sock"); // No listener — should time out. let result = wait_for_server_socket(&path, Duration::from_millis(50)); assert!(result.is_err()); assert_eq!(result.unwrap_err().kind(), io::ErrorKind::TimedOut); + let _ = std::fs::remove_dir_all(dir); } #[test] fn wait_for_server_socket_succeeds_after_delay() { - let dir = temp_socket_path("wait-delay"); + let dir = unique_test_dir("wait-delay"); std::fs::create_dir_all(&dir).unwrap(); - let path = dir.join("test.sock"); + let path = dir.join("s.sock"); // Spawn a thread that will create the listener after a short delay. let path_clone = path.clone(); @@ -292,5 +292,6 @@ mod tests { // Wait with a generous timeout — should succeed. let result = wait_for_server_socket(&path, Duration::from_secs(2)); assert!(result.is_ok()); + let _ = std::fs::remove_dir_all(dir); } } diff --git a/src/server/headless.rs b/src/server/headless.rs index d08a0ad0..9d53145b 100644 --- a/src/server/headless.rs +++ b/src/server/headless.rs @@ -91,6 +91,15 @@ const HANDSHAKE_TIMEOUT: Duration = Duration::from_secs(4); /// Maximum input payload size (bytes) for a single `ClientMessage::Input`. const MAX_INPUT_PAYLOAD: usize = 1024 * 1024; // 1 MB +/// How often the idle headless loop wakes to poll the std UnixListener for new +/// client connections. +/// +/// The listener is non-blocking and not integrated into `tokio::select!`, so +/// a low-frequency wake is required to notice new thin-client attaches while +/// otherwise idle. Keep this much slower than the old resize-poll cadence to +/// avoid reintroducing the idle CPU spin. +const CLIENT_ACCEPT_POLL_INTERVAL: Duration = Duration::from_millis(250); + // --------------------------------------------------------------------------- // Socket path helpers // --------------------------------------------------------------------------- @@ -479,7 +488,11 @@ impl HeadlessServer { } // 8. Wait for next event. - let next_deadline = self.app.next_headless_loop_deadline(now, needs_render); + let next_deadline = self + .app + .next_headless_loop_deadline(now, needs_render) + .map(|deadline| deadline.min(now + CLIENT_ACCEPT_POLL_INTERVAL)) + .or(Some(now + CLIENT_ACCEPT_POLL_INTERVAL)); let event = { tokio::select! { maybe_api = self.app.api_rx.recv() => match maybe_api { @@ -1018,6 +1031,13 @@ impl HeadlessServer { } ServerEvent::ClientInput { client_id, data } => { debug!(client_id, len = data.len(), "client input received"); + if let Some(client) = self.clients.get_mut(&client_id) { + // Ensure the next render after client input is delivered even if the + // current frame buffer still compares equal. Input can change cursor or + // PTY state asynchronously, and thin clients should not stall waiting for + // a post-input frame behind identical-frame dedupe. + client.last_frame = None; + } let events = crate::raw_input::parse_raw_input_bytes_sync(&data); let interaction = Self::events_include_interaction(&events); let foreground_changed = if interaction { @@ -1862,6 +1882,7 @@ mod tests { )); let _ = fs::create_dir_all(&dir); let socket_path = dir.join("client.sock"); + let _ = fs::remove_file(&socket_path); let listener = UnixListener::bind(&socket_path).expect("bind test listener"); listener .set_nonblocking(true) @@ -1962,8 +1983,8 @@ mod tests { #[test] fn prepare_socket_path_removes_stale_socket() { - let dir = std::env::temp_dir().join(format!( - "hs-{}-{}", + let dir = PathBuf::from(format!( + "/tmp/hs-{}-{}", std::process::id(), std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) @@ -1974,8 +1995,17 @@ mod tests { let socket_path = dir.join("stale.sock"); // Create a socket file that nobody is listening on. - let _ = UnixListener::bind(&socket_path); - // The listener goes out of scope, so the socket becomes stale. + { + let _listener = UnixListener::bind(&socket_path).expect("bind stale socket"); + } + // The listener scope ended, so the socket is now stale. + let deadline = std::time::Instant::now() + Duration::from_secs(1); + while std::time::Instant::now() < deadline { + if std::os::unix::net::UnixStream::connect(&socket_path).is_err() { + break; + } + std::thread::sleep(Duration::from_millis(10)); + } // prepare_socket_path should remove it without error. let result = prepare_socket_path(&socket_path); @@ -1989,8 +2019,8 @@ mod tests { #[test] fn prepare_socket_path_rejects_live_socket() { - let dir = std::env::temp_dir().join(format!( - "hl-{}-{}", + let dir = PathBuf::from(format!( + "/tmp/hl-{}-{}", std::process::id(), std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) diff --git a/src/update.rs b/src/update.rs index ed592ac7..1183a7eb 100644 --- a/src/update.rs +++ b/src/update.rs @@ -528,8 +528,8 @@ mod tests { .duration_since(std::time::UNIX_EPOCH) .unwrap() .as_nanos(); - std::env::temp_dir().join(format!( - "herdr-update-{name}-{}-{nanos}.sock", + std::path::PathBuf::from(format!( + "/tmp/hu-{name}-{}-{nanos}.sock", std::process::id() )) } diff --git a/tests/multi_client.rs b/tests/multi_client.rs index dee24f08..46eca891 100644 --- a/tests/multi_client.rs +++ b/tests/multi_client.rs @@ -63,7 +63,13 @@ fn cleanup_spawned_herdr(spawned: SpawnedHerdr, base: PathBuf) { fn wait_for_child_exit(child: &mut Box) { let _ = child.kill(); - let _ = child.wait(); + let deadline = Instant::now() + Duration::from_secs(5); + while Instant::now() < deadline { + if child.try_wait().ok().flatten().is_some() { + return; + } + thread::sleep(Duration::from_millis(25)); + } } fn test_lock() -> MutexGuard<'static, ()> { diff --git a/tests/support/mod.rs b/tests/support/mod.rs index 567db154..7342fde3 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -206,7 +206,13 @@ fn iter_worktree_server_pids() -> std::io::Result> { let own_pid = std::process::id(); let mut pids = Vec::new(); - for entry in fs::read_dir("/proc")? { + let proc_entries = match fs::read_dir("/proc") { + Ok(entries) => entries, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(Vec::new()), + Err(err) => return Err(err), + }; + + for entry in proc_entries { let entry = entry?; let file_name = entry.file_name(); let Some(pid) = file_name.to_str().and_then(|name| name.parse::().ok()) else {