From 951f80da2cbb3390c2bd2adb3716c1a7e7265feb Mon Sep 17 00:00:00 2001 From: Season Date: Tue, 28 Jul 2026 12:46:57 +0800 Subject: [PATCH 1/3] fix: report a clear cli error when no herdr server is running socket cli commands surfaced a raw io::Error debug string (`Error: Os { code: 2, ... }`) when nothing was listening on the api socket, which read like a bad --cwd path. map dead-socket connect failures to a `server_not_running` json error carrying the resolved socket path, printed once at the edge that surfaces the error; recovering callers (plugin offline registry fallback, agent start polling) recognize the marker and keep their existing behavior. refs #1941 --- src/cli.rs | 88 ++++++++++++++++++++++++++++++++++- src/cli/agent.rs | 7 +++ src/cli/plugin.rs | 21 +++++---- src/cli/server_not_running.rs | 60 ++++++++++++++++++++++++ src/cli/status.rs | 9 +--- src/main.rs | 8 ++++ 6 files changed, 175 insertions(+), 18 deletions(-) create mode 100644 src/cli/server_not_running.rs diff --git a/src/cli.rs b/src/cli.rs index 9d7389b517..bdbcd1c1d1 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -18,6 +18,7 @@ mod plugin; mod protocol_guard; mod runtime; mod server; +mod server_not_running; mod spec; mod status; mod tab; @@ -745,7 +746,7 @@ pub(super) fn send_request(request: &Request) -> std::io::Result std::io::Result { @@ -755,7 +756,9 @@ pub(super) fn send_request_unchecked(request: &Request) -> std::io::Result std::io::Result<()> { - let status = client.status().map_err(api_client_error_to_io)?; + let status = client + .status() + .map_err(|err| map_server_not_running_or_io(err, request_id, client))?; let server_protocol = status .protocol .ok_or_else(|| std::io::Error::other("server ping did not include a protocol version"))?; @@ -778,6 +781,49 @@ pub(crate) fn protocol_mismatch_was_reported(err: &std::io::Error) -> bool { protocol_guard::was_reported(err) } +pub(crate) fn server_not_running_was_reported(err: &std::io::Error) -> bool { + server_not_running::was_reported(err) +} + +/// Returns the `ErrorResponse` carried by a `server_not_running` marker, if any, +/// so the edge that surfaces the error can print it exactly once (deferred +/// printing: recovering callers like plugin offline fallback print nothing). +pub(crate) fn server_not_running_reported_response( + err: &std::io::Error, +) -> Option<&crate::api::schema::ErrorResponse> { + server_not_running::reported_response(err) +} + +/// True when an io::Error indicates nothing is listening on the API socket. +/// Classify by `ErrorKind` only: Windows named pipes surface different raw +/// errno values than Unix domain sockets but the same error kinds. +pub(super) fn server_not_running_error(err: &std::io::Error) -> bool { + matches!( + err.kind(), + std::io::ErrorKind::NotFound | std::io::ErrorKind::ConnectionRefused + ) +} + +/// Maps an `ApiClientError` from a socket command into the io::Error that +/// bubbles up to `main`. A dead-server connect failure is reported as a +/// friendly `server_not_running` JSON error plus a recognizable marker; all +/// other errors fall through unchanged so existing handling is preserved. +fn map_server_not_running_or_io( + err: ApiClientError, + request_id: &str, + client: &ApiClient, +) -> std::io::Error { + match err { + ApiClientError::Io(io_err) if server_not_running_error(&io_err) => { + server_not_running::reported_error(server_not_running::response( + request_id, + &client.socket_path(), + )) + } + err => api_client_error_to_io(err), + } +} + fn api_client_error_to_io(err: ApiClientError) -> std::io::Error { match err { ApiClientError::Io(err) => err, @@ -1036,4 +1082,42 @@ mod tests { "env must use KEY=VALUE" ); } + + #[test] + fn maps_dead_server_connect_failure_to_friendly_error() { + use crate::api::client::{ApiClient, ApiClientError}; + + let client = ApiClient::local(); + let socket = client.socket_path().display().to_string(); + + // The helper does NOT print; it returns a recognizable marker carrying + // the ErrorResponse so the surfacing edge can print it exactly once. + let mapped = super::map_server_not_running_or_io( + ApiClientError::Io(std::io::Error::from(std::io::ErrorKind::NotFound)), + "cli:workspace:create", + &client, + ); + + let response = super::server_not_running::reported_response(&mapped) + .expect("dead-server connect failure should carry a server_not_running response"); + assert_eq!(response.id, "cli:workspace:create"); + assert_eq!(response.error.code, "server_not_running"); + assert!(response.error.message.contains(&socket)); + + // The mapping is recognizable without string matching. + assert!(super::server_not_running::was_reported(&mapped)); + } + + #[test] + fn classifier_ignores_unrelated_io_kinds() { + use crate::api::client::{ApiClient, ApiClientError}; + + let client = ApiClient::local(); + let mapped = super::map_server_not_running_or_io( + ApiClientError::Io(std::io::Error::from(std::io::ErrorKind::TimedOut)), + "cli:workspace:create", + &client, + ); + assert!(!super::server_not_running::was_reported(&mapped)); + } } diff --git a/src/cli/agent.rs b/src/cli/agent.rs index e48d6b3bf0..e1271f4872 100644 --- a/src/cli/agent.rs +++ b/src/cli/agent.rs @@ -577,6 +577,13 @@ fn print_agent_transport_error( if super::protocol_mismatch_was_reported(&err) { return Ok(1); } + // A dead-server marker reaches here from `send_request` in the agent + // startup path; surface its deferred response exactly once instead of + // printing a second, generic transport-error line. + if let Some(response) = super::server_not_running_reported_response(&err) { + let value = serde_json::to_value(response).map_err(std::io::Error::other)?; + return super::print_response(&value); + } super::print_response(&cli_agent_error(request_id, code, err.to_string())) } diff --git a/src/cli/plugin.rs b/src/cli/plugin.rs index b4e6d51464..efdb9ecad9 100644 --- a/src/cli/plugin.rs +++ b/src/cli/plugin.rs @@ -1618,14 +1618,19 @@ fn current_unix_ms() -> u64 { } fn is_connection_error(err: &std::io::Error) -> bool { - matches!( - err.kind(), - std::io::ErrorKind::NotFound - | std::io::ErrorKind::ConnectionRefused - | std::io::ErrorKind::ConnectionAborted - | std::io::ErrorKind::ConnectionReset - | std::io::ErrorKind::BrokenPipe - ) + // A `server_not_running` marker is a connect failure for recovery purposes: + // treating it as a connection error lets plugin commands fall back to the + // offline registry. The marker carries (but does not print) a friendly + // response, so recovering here prints nothing. + super::server_not_running_was_reported(err) + || matches!( + err.kind(), + std::io::ErrorKind::NotFound + | std::io::ErrorKind::ConnectionRefused + | std::io::ErrorKind::ConnectionAborted + | std::io::ErrorKind::ConnectionReset + | std::io::ErrorKind::BrokenPipe + ) } fn print_plugin_response(method: Method) -> std::io::Result { diff --git a/src/cli/server_not_running.rs b/src/cli/server_not_running.rs new file mode 100644 index 0000000000..c05073ca17 --- /dev/null +++ b/src/cli/server_not_running.rs @@ -0,0 +1,60 @@ +use std::fmt; +use std::path::Path; + +use crate::api::schema::{ErrorBody, ErrorResponse}; + +/// Marker error signalling a dead API socket. Carries the `ErrorResponse` that +/// should be printed at the edge that finally surfaces the error, so callers +/// that recover (e.g. plugin offline fallback) print nothing. Mirrors +/// `ProtocolMismatchReported`, except printing is deferred because several CLI +/// commands recover from a dead server instead of reporting it. +#[derive(Debug)] +pub(super) struct ServerNotRunningReported { + pub(super) response: ErrorResponse, +} + +impl fmt::Display for ServerNotRunningReported { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + // Delegate to the carried message so pre-existing paths that + // stringify transport errors still show the actionable text. + f.write_str(&self.response.error.message) + } +} + +impl std::error::Error for ServerNotRunningReported {} + +/// Builds the friendly `server_not_running` ErrorResponse shown when no +/// server is listening on the resolved API socket. +pub(super) fn response(request_id: &str, socket_path: &Path) -> ErrorResponse { + ErrorResponse { + id: request_id.to_string(), + error: ErrorBody { + code: "server_not_running".into(), + message: format!( + "no herdr server is running at {}; run `herdr` to start one", + socket_path.display() + ), + }, + } +} + +/// Wraps the response in the recognizable marker WITHOUT printing. The caller +/// that ultimately surfaces the error prints the carried response (see +/// `reported_response`); recovering callers simply drop it. +pub(super) fn reported_error(response: ErrorResponse) -> std::io::Error { + std::io::Error::other(ServerNotRunningReported { response }) +} + +pub(super) fn was_reported(err: &std::io::Error) -> bool { + err.get_ref() + .and_then(|source| source.downcast_ref::()) + .is_some() +} + +/// Returns the `ErrorResponse` carried by a `server_not_running` marker, if any, +/// so the surfacing edge can print it exactly once. +pub(super) fn reported_response(err: &std::io::Error) -> Option<&ErrorResponse> { + err.get_ref() + .and_then(|source| source.downcast_ref::()) + .map(|reported| &reported.response) +} diff --git a/src/cli/status.rs b/src/cli/status.rs index 49b3c129f5..74bb3e2a15 100644 --- a/src/cli/status.rs +++ b/src/cli/status.rs @@ -159,20 +159,13 @@ fn read_server_runtime_status() -> std::io::Result { protocol: status.protocol, capabilities: status.capabilities, }), - Err(ApiClientError::Io(err)) if server_not_running_error(&err) => { + Err(ApiClientError::Io(err)) if super::server_not_running_error(&err) => { Ok(ServerRuntimeStatus::NotRunning) } Err(err) => Err(api_client_error_to_io(err)), } } -fn server_not_running_error(err: &std::io::Error) -> bool { - matches!( - err.kind(), - std::io::ErrorKind::NotFound | std::io::ErrorKind::ConnectionRefused - ) -} - fn api_client_error_to_io(err: ApiClientError) -> std::io::Error { match err { ApiClientError::Io(err) => err, diff --git a/src/main.rs b/src/main.rs index 8fc51d6b5d..7b4058d227 100644 --- a/src/main.rs +++ b/src/main.rs @@ -497,6 +497,14 @@ fn main() -> io::Result<()> { Ok(cli::CommandOutcome::Handled(code)) => std::process::exit(code), Ok(cli::CommandOutcome::NotCli) => {} Err(err) if cli::protocol_mismatch_was_reported(&err) => std::process::exit(1), + Err(err) if cli::server_not_running_was_reported(&err) => { + if let Some(response) = cli::server_not_running_reported_response(&err) { + if let Ok(json) = serde_json::to_string(response) { + eprintln!("{json}"); + } + } + std::process::exit(1); + } Err(err) => return Err(err), } From eeccea3dc0d1620f06f42b0ae8756284cd68de7a Mon Sep 17 00:00:00 2001 From: Season Date: Tue, 28 Jul 2026 12:59:49 +0800 Subject: [PATCH 2/3] fix: map dead-socket errors on the unchecked cli request path refs #1941 --- src/cli.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index bdbcd1c1d1..4d3c67c079 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -750,9 +750,10 @@ pub(super) fn send_request(request: &Request) -> std::io::Result std::io::Result { - ApiClient::local() + let client = ApiClient::local(); + client .request_value(request) - .map_err(api_client_error_to_io) + .map_err(|err| map_server_not_running_or_io(err, &request.id, &client)) } fn ensure_server_protocol_compatible(client: &ApiClient, request_id: &str) -> std::io::Result<()> { From a98beadeb1d97fa14deda9da6513ee74657dfad3 Mon Sep 17 00:00:00 2001 From: Season Saw Date: Sat, 1 Aug 2026 15:00:20 +0800 Subject: [PATCH 3/3] fix: make server-not-running guidance session-aware refs #1941 --- src/cli/server_not_running.rs | 16 ++++++++- tests/cli/sessions.rs | 63 +++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/src/cli/server_not_running.rs b/src/cli/server_not_running.rs index c05073ca17..9e974f3c53 100644 --- a/src/cli/server_not_running.rs +++ b/src/cli/server_not_running.rs @@ -26,18 +26,32 @@ impl std::error::Error for ServerNotRunningReported {} /// Builds the friendly `server_not_running` ErrorResponse shown when no /// server is listening on the resolved API socket. pub(super) fn response(request_id: &str, socket_path: &Path) -> ErrorResponse { + let attach_command = startup_command(socket_path); ErrorResponse { id: request_id.to_string(), error: ErrorBody { code: "server_not_running".into(), message: format!( - "no herdr server is running at {}; run `herdr` to start one", + "no herdr server is running at {}; run `{attach_command}` to start or attach it", socket_path.display() ), }, } } +fn startup_command(socket_path: &Path) -> String { + let session_socket = + crate::session::api_socket_path_for(crate::session::active_name().as_deref()); + if socket_path == session_socket { + crate::session::local_attach_command() + } else { + // A socket override wins over an inherited HERDR_SESSION. Keep the + // command in the current environment so it starts the overridden + // target instead of directing the user to an unrelated session. + "herdr".to_string() + } +} + /// Wraps the response in the recognizable marker WITHOUT printing. The caller /// that ultimately surfaces the error prints the carried response (see /// `reported_response`); recovering callers simply drop it. diff --git a/tests/cli/sessions.rs b/tests/cli/sessions.rs index 8cb0f4b799..ae7b830610 100644 --- a/tests/cli/sessions.rs +++ b/tests/cli/sessions.rs @@ -184,6 +184,69 @@ fn named_sessions_use_separate_servers_and_workspace_state() { cleanup_test_base(&base); } +#[test] +fn dead_server_cli_reports_one_session_aware_json_line() { + fn assert_server_not_running( + output: std::process::Output, + socket_path: &Path, + attach_command: &str, + ) { + assert_eq!( + output.status.code(), + Some(1), + "stdout={} stderr={}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + assert!(output.stdout.is_empty(), "server errors belong on stderr"); + + let stderr = String::from_utf8(output.stderr).unwrap(); + let lines: Vec<_> = stderr.lines().collect(); + assert_eq!(lines.len(), 1, "expected exactly one JSON line: {stderr:?}"); + + let response: serde_json::Value = serde_json::from_str(lines[0]).unwrap(); + assert_eq!(response["id"], "cli:workspace:create"); + assert_eq!(response["error"]["code"], "server_not_running"); + assert_eq!( + response["error"]["message"], + format!( + "no herdr server is running at {}; run `{attach_command}` to start or attach it", + socket_path.display() + ) + ); + } + + let base = unique_test_dir(); + let config_home = base.join("config"); + let runtime_dir = base.join("runtime"); + fs::create_dir_all(&runtime_dir).unwrap(); + register_runtime_dir(&runtime_dir); + + let named_socket = named_session_socket(&config_home, "foo"); + let missing = run_named_cli( + &config_home, + &runtime_dir, + &["--session", "foo", "workspace", "create"], + ); + assert_server_not_running(missing, &named_socket, "herdr session attach foo"); + + let stale_socket = runtime_dir.join("stale.sock"); + drop(UnixListener::bind(&stale_socket).unwrap()); + let stale = Command::new(env!("CARGO_BIN_EXE_herdr")) + .args(["workspace", "create"]) + .env("XDG_CONFIG_HOME", &config_home) + .env("XDG_RUNTIME_DIR", &runtime_dir) + .env("HERDR_SOCKET_PATH", &stale_socket) + .env("HERDR_SESSION", "unrelated") + .env_remove("HERDR_CLIENT_SOCKET_PATH") + .env_remove("HERDR_ENV") + .output() + .unwrap(); + assert_server_not_running(stale, &stale_socket, "herdr"); + + cleanup_test_base(&base); +} + #[test] fn integration_commands_run_locally_when_server_is_missing() { let base = unique_test_dir();