fix: report a real exit code from bssh ping

ARCHITECTURE.md

@@ -179,6 +179,8 @@ MPI-compatible exit code handling:
 - Automatic main rank detection (Backend.AI integration)
 - Preserves actual exit codes (SIGSEGV=139, OOM=137, etc.)
 
+`ping` is the exception: it runs no user command, so no remote status exists to forward and `MainRank` does not apply. It reuses `RequireAllSuccess` for the 0/1 boundary and reports 255 when no host answered or when bssh failed before connecting, following OpenSSH's "ssh itself failed" convention. `ping_nodes` returns a `PingOutcome` tally rather than `Result<()>`, `dispatch_command` returns `Result<i32>`, and `main::dispatch_and_exit` is the single place that converts a nonzero command-level code into the process exit status.
+
 ### Shared Module
 
 Common utilities for code reuse between bssh client and server implementations:
@@ -906,6 +908,7 @@ See [docs/architecture/configuration.md](./docs/architecture/configuration.md) f
 - **0**: Success (all nodes, or main rank succeeded)
 - **1**: General failure
 - **130**: Terminated by SIGINT (Ctrl+C)
+- **255**: Generated by `ping` as a bssh-level signal when no host was reachable, or bssh failed before attempting any connection. The default `MainRank` strategy can also produce 255 by forwarding a remote command's exit status verbatim
 - **Other**: Preserved from main rank (SIGSEGV=139, OOM=137, etc.)
 
 See [docs/architecture/exit-code-strategy.md](./docs/architecture/exit-code-strategy.md) for detailed strategy documentation.

CHANGELOG.md

@@ -14,13 +14,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **Honor known_hosts `@revoked`/`@cert-authority` marker lines, match hostnames case-insensitively, and close the window between creating `~/.ssh`/`known_hosts` and restricting their permissions** (#239). russh's known_hosts parser reads a marker line's own `@revoked`/`@cert-authority` token as the literal host field, so it never matched the real host and both accept-new and strict mode silently ignored these lines, meaning a key an operator had explicitly revoked was recorded and accepted like any ordinary first use. A dedicated scan now runs before the ordinary lookup in every known_hosts-based mode: a `@revoked` line whose host matches (the exact, comma-list, or `*`/`?` glob form, erring toward matching when in doubt since failing to honor a revocation is worse than an unnecessary rejection) and whose key equals the offered key is a hard rejection with a new `HostKeyRevoked` error, while a matching `@revoked` line naming a *different* key does not block the offered one; a `@cert-authority` match only prints a loud warning, since bssh has no CA signature validation to fall back to and failing closed there would break every working CA setup with no workaround, and falls through to ordinary TOFU. Hostname matching is also normalized to lowercase once at the point of verification, for both the lookup and the recorded entry, matching OpenSSH's case-insensitive known_hosts comparison (russh's own matcher is a plain byte compare), so `NODE1.example.com` and `node1.example.com` now hit the same pin instead of the differently-cased spelling looking unknown and re-recording. Finally, `~/.ssh` and `known_hosts` are created with mode 0700/0600 directly (`DirBuilder`/`OpenOptions` with an explicit mode) before `learn_known_hosts_path` runs, rather than letting it create them with the process umask and tightening the mode only afterward, closing the brief window in which an unusually permissive umask could leave either one group- or world-writable.
 
 ### Changed
+- **Breaking, lib API: `bssh::commands::ping::ping_nodes` now returns `Result<PingOutcome>` instead of `Result<()>`** (#245). The per-host tally has to survive the return for the caller to compute an exit code from it. `PingOutcome` carries `total`, `succeeded`, and `failed`, and `PingOutcome::exit_code()` applies the 0/1/255 contract. Callers that ignored the old `Ok(())` keep compiling; callers that matched on it exhaustively update the pattern.
 - **Breaking, lib API: address family selection added a required `AddressFamily` parameter to several public library functions** (#246). Every call site needed the new value anyway, so the maintainer chose a source break over back-compat wrapper overloads; consumers of the `bssh` library crate update call sites when picking up this version.
   - `ForwardingSpec::parse_local`, `parse_dynamic`, and `parse` now take an `AddressFamily` argument.
   - The four public `SshClient::*_with_jump_hosts` helpers (`upload_file_with_jump_hosts`, `download_file_with_jump_hosts`, `upload_dir_with_jump_hosts`, `download_dir_with_jump_hosts`) likewise gained a required `AddressFamily` argument.
   - `ForwardingConfig` gained a public `address_family` field.
   - `bssh::ssh::tokio_client::Error` gained a `NoAddressForFamily` variant.
 
 ### Fixed
+- **Make `bssh ping` report an exit code, instead of always exiting 0 while its help text promised otherwise** (#245). `ping_nodes` counted successes and failures only to print them, returned `Ok(())` unconditionally, and nothing downstream translated the counts, so `bssh -H unreachable-host ping; echo $?` printed 0 and every script that branched on `bssh ping` succeeding passed unconditionally. Ping now follows a 0/1/255 contract: 0 when every targeted host connected and authenticated, 1 when at least one host was reachable and at least one failed, and 255 when no host succeeded or when bssh failed before it could attempt any connection (configuration file that could not be loaded, host list that resolved to nothing). The 0/1 boundary is `ExitCodeStrategy::RequireAllSuccess`, since a health check is green only when every node is green; `MainRank` does not apply because ping runs no user command whose status could be forwarded. The 255 value follows OpenSSH, which reserves it for "ssh itself encountered an error", and it keeps a partially degraded cluster distinct from one that could not be reached at all. This is a user-visible behavior change in both directions: the all-unreachable case previously exited 0 and now exits 255, which is also not the 1 the old help text implied. Command-line usage errors are still rejected by the argument parser with its own exit code 2, unchanged and identical across subcommands. The `ping` help text also claimed it "Reports connection status, authentication success, and response times" while printing no timing at all; the claim is removed rather than implemented, and per-host timing remains a separate proposal.
 - **Wire up the `-4` / `-6` address family flags and the ssh_config `AddressFamily` keyword, which were parsed and then discarded** (#246). Both were advertised in `--help` and the man page and had no effect: `bssh -6` against a dual-stack host could still connect over IPv4, and `AddressFamily any|inet|inet6` round-tripped through the config layer without ever being read. A single `AddressFamily` preference is now resolved once per dispatch path with OpenSSH precedence (command line flag over config keyword over the `any` default), carried on `SshConnectionConfig`, and applied when the connect path filters resolved addresses. It covers direct connections for `exec`, interactive sessions, `ping`, and SFTP `upload`/`download`, plus the first hop of a `-J` chain, which shares that path. Forcing a family that has no resolved address is a hard failure with no fallback to the other family, matching OpenSSH, reported as `no IPv6 address found for <host>` (or the IPv4 equivalent) instead of the generic `could not resolve to any addresses`. An unrecognized `AddressFamily` value warns and is treated as `any` rather than failing a configuration file OpenSSH would accept. With neither the flag nor the keyword set, every resolved address is still tried in resolver order, unchanged.
 - **Apply the address family preference to port forwarding, changing the default `-L`/`-D` listen address under `-6`** (#246). This is a user-visible default change: with `-6`, a `-L` or `-D` specification that does not name a bind address now listens on `::1` instead of `127.0.0.1`, and the `*:port` wildcard form listens on `::` instead of `0.0.0.0`. A specification that names a bind address explicitly is unaffected, as are `-4` and the no-flag default, which both keep the IPv4 loopback. Scripts that pass `-6` and assume an IPv4 loopback listener should name the bind address explicitly (for example `-L 127.0.0.1:8080:example.com:80`). Forwarding *targets* are also filtered by the forced family for `-L` and SOCKS5 `-D` requests, but the remote server performs the actual connect, so that filtering is a best-effort hint; SOCKS4 requests carry a literal IPv4 destination by protocol definition and are passed through unfiltered. The `-R` listener lives on the server and is not governed by the local flag. Jump hops past the first one still resolve their target locally through the same `direct-tcpip` mechanism, but this change does not yet apply the family filter there (scope limitation, tracked as issue #248); the family is still honored when selecting the address recorded for host key verification on those hops. The man page documents the full scope, including this limitation.
 - **Show the full error context chain when an interactive connection fails** (#238). Interactive mode printed only anyhow's outermost context, so a jump-host failure surfaced as `Failed to establish jump host connection to <host>` with no indication of which hop failed or why, leaving first-jump authentication failures, destination TCP refusals, and destination authentication failures indistinguishable. Both interactive execution paths (PTY and traditional) and the directory-download failure message now render the whole chain through a shared `format_connection_error` helper that uses anyhow's alternate `Display` form, matching the format the parallel exec path already used. Tracing verbosity and exec-mode output are unchanged.

docs/architecture/exit-code-strategy.md

@@ -8,6 +8,7 @@
 - [Main Rank Detection](#main-rank-detection)
 - [Exit Code Strategies](#exit-code-strategies)
 - [Strategy Comparison](#strategy-comparison)
+- [The ping Contract](#the-ping-contract)
 - [Implementation Details](#implementation-details)
 - [Migration Guide](#migration-guide)
 
@@ -194,6 +195,39 @@ ExitCodeStrategy::MainRankWithFailureCheck => {
 
 **Bold** values show where strategies differ.
 
+## The ping Contract
+
+`bssh ping` does not use `ExitCodeStrategy` directly, because it runs no user command whose status could be forwarded. `MainRank` is therefore meaningless for it. Instead it reuses the `RequireAllSuccess` semantics for the 0/1 boundary (a health check is green only when every host is green) and adds one value it generates itself as a bssh-level signal: 255. The exec path can still report 255, but only when the default `MainRank` strategy forwards that exact status from a remote command.
+
+| Scenario | Exit code |
+|----------|-----------|
+| Every targeted host connected and authenticated | 0 |
+| At least one host reachable, at least one failed | 1 |
+| No host succeeded | 255 |
+| bssh failed before attempting any connection (config load failure, no host resolved) | 255 |
+
+**Why 255**: OpenSSH reserves 255 for "ssh itself encountered an error" as opposed to a status forwarded from the remote command. Since ping has no remote command, every total failure is by definition an ssh-level failure. The split lets a caller distinguish a partially degraded cluster (1) from one it could not reach at all, or could not even start against (255).
+
+**Implementation**: `src/commands/ping.rs` returns a `PingOutcome { total, succeeded, failed }` instead of `Result<()>`, and `PingOutcome::exit_code()` applies the table above:
+
+```rust
+pub fn exit_code(&self) -> i32 {
+    if self.succeeded == 0 {
+        PING_SSH_LEVEL_FAILURE // 255, including an empty host list
+    } else if self.failed > 0 {
+        1 // RequireAllSuccess: any failure is a failure
+    } else {
+        0
+    }
+}
+```
+
+A unit test asserts that `PingOutcome::exit_code()` equals `ExitCodeStrategy::RequireAllSuccess.calculate()` for every case where at least one host answered, so the two cannot drift apart.
+
+**Error propagation**: an `Err` returned from the ping path is a hard failure raised before any per-host tally exists, so it maps to 255 rather than to the 1 that means "some hosts answered and some did not". `main::map_hard_failure` applies that mapping on the ping path only; every other subcommand keeps the default, where returning `Err` from `main` prints the chain and exits 1. Command-line usage errors are rejected by clap before this contract applies and keep clap's own exit code 2, as they do for every subcommand.
+
+**Exit code plumbing**: `dispatch_command` returns `Result<i32>` and `main::dispatch_and_exit` is the single place that turns a nonzero command-level code into the process exit status. The `exec` path predates this and still calls `std::process::exit` itself from `src/commands/exec.rs` after applying its `ExitCodeStrategy`.
+
 ## Implementation Details
 
 ### File Structure
@@ -207,7 +241,11 @@ src/executor/
 └── parallel.rs # Marks main rank in results
 
 src/commands/
-└── exec.rs # Applies exit strategy based on CLI flags
+├── exec.rs # Applies exit strategy based on CLI flags
+└── ping.rs # PingOutcome and the 0/1/255 ping contract
+
+src/app/
+└── dispatcher.rs # Returns the command-level exit code to main
 
 src/
 └── cli.rs # CLI flags: --require-all-success, --check-all-nodes

docs/man/bssh.1

@@ -1867,6 +1867,34 @@ Main rank succeeded but other nodes failed
 .B 2-255
 Main rank failed with this exit code
 
+.SS ping Subcommand
+The
+.B ping
+subcommand runs no remote command, so there is no remote exit status to
+forward and the main rank strategy does not apply. Its 0/1 boundary follows
+.B --require-all-success
+(a health check is green only when every host is green), and total failure is
+reported as 255 following OpenSSH's convention that 255 means ssh itself
+encountered an error:
+
+.TP
+.B 0
+Every targeted host connected and authenticated
+
+.TP
+.B 1
+At least one host was reachable and at least one failed
+
+.TP
+.B 255
+No host was reachable, or bssh failed before it could attempt any connection
+(configuration file could not be loaded, no host resolved, and similar)
+
+.PP
+Command-line usage errors are rejected by the argument parser before this
+contract applies and keep the parser's own exit code 2, as they do for every
+subcommand.
+
 .SH OUTPUT FILES
 When using the
 .B --output-dir

src/app/dispatcher.rs

@@ -40,6 +40,10 @@ use super::initialization::determine_use_keychain;
 use super::initialization::{AppContext, determine_ssh_key_path};
 use super::utils::format_duration;
 
+/// Exit code for a subcommand that completed without a per-host failure to
+/// report.
+const EXIT_SUCCESS: i32 = 0;
+
 /// Build SSH connection config with keepalive, compression, and address
 /// family settings.
 /// Precedence: CLI > SSH config > YAML config > defaults.
@@ -154,8 +158,14 @@ fn subcommand_name(command: &Option<Commands>) -> &'static str {
     }
 }
 
-/// Dispatch commands to their appropriate handlers
-pub async fn dispatch_command(cli: &Cli, ctx: &AppContext) -> Result<()> {
+/// Dispatch commands to their appropriate handlers.
+///
+/// Returns the exit code the process should report. Most subcommands either
+/// succeed (0) or return `Err`, but `ping` completes normally while still
+/// having per-host failures to report, so the count has to survive the return.
+/// The caller (`main`) is the single place that turns a nonzero value into the
+/// process exit status.
+pub async fn dispatch_command(cli: &Cli, ctx: &AppContext) -> Result<i32> {
     // Get command to execute
     let command = cli.get_command();
 
@@ -219,7 +229,7 @@ pub async fn dispatch_command(cli: &Cli, ctx: &AppContext) -> Result<()> {
     match &cli.command {
         Some(Commands::List) => {
             list_clusters(&ctx.config);
-            Ok(())
+            Ok(EXIT_SUCCESS)
         }
         Some(Commands::Ping) => {
             let key_path = determine_ssh_key_path(
@@ -247,7 +257,10 @@ pub async fn dispatch_command(cli: &Cli, ctx: &AppContext) -> Result<()> {
                 ctx.cluster_name.as_deref().or(cli.cluster.as_deref()),
             );
 
-            ping_nodes(
+            // `ping` is the one subcommand whose exit code depends on how many
+            // hosts answered, so the outcome is translated here rather than
+            // discarded. See `PingOutcome` for the 0/1/255 contract.
+            let outcome = ping_nodes(
                 ctx.nodes.clone(),
                 ctx.max_parallel,
                 key_path.as_deref(),
@@ -262,7 +275,9 @@ pub async fn dispatch_command(cli: &Cli, ctx: &AppContext) -> Result<()> {
                 ssh_password.clone(),
                 ssh_connection_config,
             )
-            .await
+            .await?;
+
+            Ok(outcome.exit_code())
         }
         Some(Commands::Upload {
             source,
@@ -300,7 +315,8 @@ pub async fn dispatch_command(cli: &Cli, ctx: &AppContext) -> Result<()> {
                     hostname_for_ssh_config.as_deref(),
                 ),
             };
-            upload_file(params, source, destination).await
+            upload_file(params, source, destination).await?;
+            Ok(EXIT_SUCCESS)
         }
         Some(Commands::Download {
             source,
@@ -338,7 +354,8 @@ pub async fn dispatch_command(cli: &Cli, ctx: &AppContext) -> Result<()> {
                     hostname_for_ssh_config.as_deref(),
                 ),
             };
-            download_file(params, source, destination).await
+            download_file(params, source, destination).await?;
+            Ok(EXIT_SUCCESS)
         }
         Some(Commands::Interactive {
             single_node,
@@ -357,15 +374,20 @@ pub async fn dispatch_command(cli: &Cli, ctx: &AppContext) -> Result<()> {
                 work_dir.as_deref(),
                 ssh_password.clone(),
             )
-            .await
+            .await?;
+            Ok(EXIT_SUCCESS)
         }
         Some(Commands::CacheStats { .. }) => {
             // This is handled in main.rs before node resolution
             unreachable!("CacheStats should be handled before dispatch")
         }
         None => {
-            // Execute command (auto-exec or interactive shell)
-            handle_exec_command(cli, ctx, &command, ssh_password.clone()).await
+            // Execute command (auto-exec or interactive shell). This path owns
+            // its own exit code strategy (`ExitCodeStrategy`, selected by
+            // `--require-all-success` / `--check-all-nodes`) and exits the
+            // process itself when the strategy yields a nonzero code.
+            handle_exec_command(cli, ctx, &command, ssh_password.clone()).await?;
+            Ok(EXIT_SUCCESS)
         }
     }
 }

src/cli/bssh.rs

@@ -340,7 +340,7 @@ pub enum Commands {
 
     #[command(
         about = "Test connectivity to hosts",
-        long_about = "Verifies SSH connectivity and authentication to all target hosts.\nReports connection status, authentication success, and response times.\nUseful for validating cluster configuration and SSH key setup.\n\nExit codes: 0 (all reachable), 1 (any unreachable)"
+        long_about = "Verifies SSH connectivity and authentication to all target hosts.\nReports per-host connection status and the reason each failure occurred.\nUseful for validating cluster configuration and SSH key setup.\n\nExit codes:\n  0    every host connected and authenticated\n  1    at least one host reachable, at least one unreachable\n  255  no host reachable, or bssh failed before connecting"
     )]
     Ping,