From 1daf54ecbf960c42d612ed4ad2764045a606fb30 Mon Sep 17 00:00:00 2001 From: Golden-Pigeon Date: Fri, 22 May 2026 02:39:02 +0300 Subject: feat: show named session command hints refs #199 --- src/client/mod.rs | 115 ++++++++++++++++++++++++++++++++++++++++++++++- src/server/autodetect.rs | 53 ++++++++++++++++++++-- src/session.rs | 52 +++++++++++++++++++++ src/update.rs | 61 ++++++++++++++++++++----- 4 files changed, 266 insertions(+), 15 deletions(-) (limited to 'src') diff --git a/src/client/mod.rs b/src/client/mod.rs index 11273061..1ac05201 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -154,7 +154,11 @@ impl std::fmt::Display for ClientError { write!(f, "\nRun `{reattach_command}` to reattach")?; } else { write!(f, "detached from server")?; - write!(f, "\nRun `herdr` to reattach")?; + write!( + f, + "\nRun `{}` to reattach", + crate::session::local_attach_command() + )?; } } _ => { @@ -1125,6 +1129,65 @@ fn init_logging() { #[cfg(test)] mod tests { use super::*; + use std::ffi::OsString; + use std::sync::{Mutex, OnceLock}; + + fn env_lock() -> &'static Mutex<()> { + static LOCK: OnceLock> = OnceLock::new(); + LOCK.get_or_init(|| Mutex::new(())) + } + + fn restore_env_var(key: &str, value: Option) { + if let Some(value) = value { + std::env::set_var(key, value); + } else { + std::env::remove_var(key); + } + } + + struct EnvVarGuard { + key: &'static str, + previous: Option, + } + + impl EnvVarGuard { + fn set(key: &'static str, value: &str) -> Self { + let previous = std::env::var_os(key); + std::env::set_var(key, value); + Self { key, previous } + } + } + + impl Drop for EnvVarGuard { + fn drop(&mut self) { + restore_env_var(self.key, self.previous.clone()); + } + } + + struct EnvVarsRemovedGuard { + previous: Vec<(&'static str, Option)>, + } + + impl EnvVarsRemovedGuard { + fn new(keys: &[&'static str]) -> Self { + let previous: Vec<_> = keys + .iter() + .map(|key| (*key, std::env::var_os(key))) + .collect(); + for key in keys { + std::env::remove_var(key); + } + Self { previous } + } + } + + impl Drop for EnvVarsRemovedGuard { + fn drop(&mut self) { + for (key, value) in self.previous.clone() { + restore_env_var(key, value); + } + } + } #[test] fn clipboard_image_paste_bridge_triggers_on_ctrl_v_and_empty_paste() { @@ -1295,6 +1358,56 @@ mod tests { ); } + #[test] + fn client_error_display_detached_default_session_reattach_hint() { + let _guard = env_lock().lock().unwrap(); + let _env = EnvVarsRemovedGuard::new(&[ + crate::remote::REATTACH_COMMAND_ENV_VAR, + crate::session::SESSION_ENV_VAR, + ]); + let err = ClientError::ServerShutdown { + reason: Some("detached".into()), + }; + let msg = err.to_string(); + assert!( + msg.contains("Run `herdr` to reattach"), + "should suggest default reattach command: {msg}" + ); + } + + #[test] + fn client_error_display_detached_named_session_reattach_hint() { + let _guard = env_lock().lock().unwrap(); + let _remote_env = EnvVarsRemovedGuard::new(&[crate::remote::REATTACH_COMMAND_ENV_VAR]); + let _session_env = EnvVarGuard::set(crate::session::SESSION_ENV_VAR, "work"); + let err = ClientError::ServerShutdown { + reason: Some("detached".into()), + }; + let msg = err.to_string(); + assert!( + msg.contains("Run `herdr session attach work` to reattach"), + "should suggest named session reattach command: {msg}" + ); + } + + #[test] + fn client_error_display_detached_remote_reattach_hint_takes_precedence() { + let _guard = env_lock().lock().unwrap(); + let _remote_env = EnvVarGuard::set( + crate::remote::REATTACH_COMMAND_ENV_VAR, + "herdr --remote host --session work", + ); + let _session_env = EnvVarGuard::set(crate::session::SESSION_ENV_VAR, "work"); + let err = ClientError::ServerShutdown { + reason: Some("detached".into()), + }; + let msg = err.to_string(); + assert!( + msg.contains("Run `herdr --remote host --session work` to reattach"), + "should prefer remote reattach command: {msg}" + ); + } + #[test] fn client_error_display_connection_lost() { let err = diff --git a/src/server/autodetect.rs b/src/server/autodetect.rs index 976d2583..c278329d 100644 --- a/src/server/autodetect.rs +++ b/src/server/autodetect.rs @@ -85,10 +85,12 @@ fn read_server_status() -> io::Result> { } fn validate_running_server_compatibility() -> io::Result<()> { + let stop_command = crate::session::local_stop_command(); + let attach_command = crate::session::local_attach_command(); let Some(status) = read_server_status()? else { - return Err(io::Error::other( - "a herdr server is listening, but its status API is unavailable. Try `herdr server stop`; if that fails, stop the old server process manually, then run `herdr` again.", - )); + return Err(io::Error::other(format!( + "a herdr server is listening, but its status API is unavailable. Try `{stop_command}`; if that fails, stop the old server process manually, then run `{attach_command}` again." + ))); }; if status.protocol == Some(crate::protocol::PROTOCOL_VERSION) { @@ -96,7 +98,7 @@ fn validate_running_server_compatibility() -> io::Result<()> { } Err(io::Error::other(format!( - "herdr server is running from v{} / protocol {}, but this client is v{} / protocol {}.\nStop the old server with `herdr server stop`, then run `herdr` again.", + "herdr server is running from v{} / protocol {}, but this client is v{} / protocol {}.\nStop the old server with `{stop_command}`, then run `{attach_command}` again.", status.version.as_deref().unwrap_or("unknown"), status .protocol @@ -425,4 +427,47 @@ mod tests { std::env::remove_var(crate::api::SOCKET_PATH_ENV_VAR); let _ = std::fs::remove_dir_all(dir); } + + #[test] + fn validate_running_server_compatibility_names_session_commands_for_protocol_mismatch() { + let _guard = env_lock().lock().unwrap(); + let dir = unique_test_dir("named-protocol"); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("api.sock"); + let listener = UnixListener::bind(&path).unwrap(); + std::env::set_var(crate::api::SOCKET_PATH_ENV_VAR, &path); + std::env::set_var(crate::session::SESSION_ENV_VAR, "work"); + crate::session::clear_explicit_session_for_test(); + let handle = std::thread::spawn(move || { + let (mut stream, _) = listener.accept().unwrap(); + let mut request = String::new(); + BufReader::new(stream.try_clone().unwrap()) + .read_line(&mut request) + .unwrap(); + assert!(request.contains("ping")); + let body = format!( + "{{\"id\":\"autodetect:server:status\",\"result\":{{\"type\":\"pong\",\"version\":\"0.5.5\",\"protocol\":{}}}}}\n", + crate::protocol::PROTOCOL_VERSION + 1 + ); + stream.write_all(body.as_bytes()).unwrap(); + stream.flush().unwrap(); + }); + + let err = validate_running_server_compatibility().unwrap_err(); + let message = err.to_string(); + + let _ = handle.join(); + assert!( + message.contains("Stop the old server with `herdr session stop work`"), + "unexpected error: {message}" + ); + assert!( + message.contains("then run `herdr session attach work` again"), + "unexpected error: {message}" + ); + std::env::remove_var(crate::api::SOCKET_PATH_ENV_VAR); + std::env::remove_var(crate::session::SESSION_ENV_VAR); + crate::session::clear_explicit_session_for_test(); + let _ = std::fs::remove_dir_all(dir); + } } diff --git a/src/session.rs b/src/session.rs index b164fecc..f00797c0 100644 --- a/src/session.rs +++ b/src/session.rs @@ -92,6 +92,20 @@ pub fn active_name() -> Option { .filter(|name| validate_name(name).is_ok()) } +pub fn local_attach_command() -> String { + match active_name() { + Some(name) => format!("herdr session attach {name}"), + None => "herdr".to_string(), + } +} + +pub fn local_stop_command() -> String { + match active_name() { + Some(name) => format!("herdr session stop {name}"), + None => "herdr server stop".to_string(), + } +} + pub fn explicit_session_requested() -> bool { EXPLICIT_SESSION_REQUESTED.load(Ordering::Relaxed) } @@ -475,6 +489,44 @@ mod tests { clear_explicit_session_for_test(); } + #[test] + fn local_attach_command_uses_default_launch_for_default_session() { + let _guard = env_lock().lock().unwrap(); + std::env::remove_var(SESSION_ENV_VAR); + + assert_eq!(local_attach_command(), "herdr"); + } + + #[test] + fn local_attach_command_uses_session_attach_for_named_session() { + let _guard = env_lock().lock().unwrap(); + std::env::set_var(SESSION_ENV_VAR, "work"); + + assert_eq!(local_attach_command(), "herdr session attach work"); + + std::env::remove_var(SESSION_ENV_VAR); + } + + #[test] + fn local_stop_command_uses_server_stop_for_default_session() { + let _guard = env_lock().lock().unwrap(); + std::env::remove_var(SESSION_ENV_VAR); + + assert_eq!(local_stop_command(), "herdr server stop"); + + std::env::remove_var(SESSION_ENV_VAR); + } + + #[test] + fn local_stop_command_uses_session_stop_for_named_session() { + let _guard = env_lock().lock().unwrap(); + std::env::set_var(SESSION_ENV_VAR, "work"); + + assert_eq!(local_stop_command(), "herdr session stop work"); + + std::env::remove_var(SESSION_ENV_VAR); + } + #[test] fn explicit_session_socket_ignores_inherited_socket_override() { let _guard = env_lock().lock().unwrap(); diff --git a/src/update.rs b/src/update.rs index 98943424..f37a9265 100644 --- a/src/update.rs +++ b/src/update.rs @@ -462,8 +462,9 @@ fn prompt_to_stop_server_before_update( if !io::stdin().is_terminal() { if requires_stop { return Err(format!( - "a herdr server is running and updating to v{} requires stopping it; run `herdr server stop`, then run `herdr update` again", - release.version + "a herdr server is running and updating to v{} requires stopping it; run `{}`, then run `herdr update` again", + release.version, + crate::session::local_stop_command() )); } @@ -533,10 +534,10 @@ fn plan_running_server_update( ) -> Result, String> { let Some(server) = read_running_server_info()? else { if client_protocol_server_is_running() { - return Err( - "a herdr server is listening, but its status API is unavailable; try `herdr server stop`, or stop the old server process manually, then run `herdr update` again" - .to_string(), - ); + return Err(format!( + "a herdr server is listening, but its status API is unavailable; try `{}`, or stop the old server process manually, then run `herdr update` again", + crate::session::local_stop_command() + )); } return Ok(None); }; @@ -560,10 +561,10 @@ fn stop_running_server_for_update( prompt_to_stop_server_before_update(&plan.server, release, plan.requires_stop)?; if !stop_server { if plan.requires_stop { - return Err( - "update cancelled; stop the running herdr server with `herdr server stop`, then run `herdr update` again" - .to_string(), - ); + return Err(format!( + "update cancelled; stop the running herdr server with `{}`, then run `herdr update` again", + crate::session::local_stop_command() + )); } return Ok(false); } @@ -1185,8 +1186,14 @@ mod tests { atomic::{AtomicBool, Ordering}, Arc, }; + use std::sync::{Mutex, OnceLock}; use std::thread; + fn env_lock() -> &'static Mutex<()> { + static LOCK: OnceLock> = OnceLock::new(); + LOCK.get_or_init(|| Mutex::new(())) + } + fn unique_test_socket_path(name: &str) -> std::path::PathBuf { let nanos = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) @@ -1355,6 +1362,40 @@ mod tests { assert!(update_requires_server_stop(&server, &unknown_release)); } + #[test] + fn noninteractive_update_requires_stop_names_session_stop_command() { + let _guard = env_lock().lock().unwrap(); + assert!( + !io::stdin().is_terminal(), + "this test relies on noninteractive test stdin" + ); + std::env::set_var(crate::session::SESSION_ENV_VAR, "work"); + crate::session::clear_explicit_session_for_test(); + let server = crate::api::RuntimeStatus { + version: Some("0.5.5".to_string()), + protocol: Some(2), + }; + let release = ReleaseInfo { + version: Version::parse("0.5.6").unwrap(), + target_protocol: Some(3), + download_url: "https://example.com/herdr".to_string(), + notes_body: "### Changed\n- One".to_string(), + }; + + let err = prompt_to_stop_server_before_update(&server, &release, true).unwrap_err(); + + assert!( + err.contains("run `herdr session stop work`"), + "unexpected error: {err}" + ); + assert!( + err.contains("then run `herdr update` again"), + "unexpected error: {err}" + ); + std::env::remove_var(crate::session::SESSION_ENV_VAR); + crate::session::clear_explicit_session_for_test(); + } + #[test] fn client_protocol_server_is_running_at_detects_live_socket() { let socket_path = unique_test_socket_path("client-live"); -- cgit v1.2.3