fix: report a real exit code from bssh ping

[inureyes]

Summary

bssh ping always exited 0, even when every target host was unreachable, while its help text promised 0 (all reachable), 1 (any unreachable). ping_nodes counted per-host successes and failures only to print them, returned Ok(()) unconditionally, and neither the dispatcher nor main translated anything, so every script branching on bssh ping succeeding passed unconditionally. This implements the 0/1/255 mapping signed off on the issue.

Exit code contract

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

The 0/1 boundary aligns with ExitCodeStrategy::RequireAllSuccess: ping is a health check, so it is green only when every node is green. ExitCodeStrategy::MainRank does not apply, because ping runs no user command whose status could be forwarded. 255 follows OpenSSH, which reserves it for "ssh itself encountered an error"; since ping has no remote command, every total failure is by definition an ssh-level failure, and the split lets a caller tell a partially degraded cluster apart from one it could not reach at all.

How the exit code is wired through

ping_nodes returns Result<PingOutcome> instead of Result<()>. PingOutcome { total, succeeded, failed } carries the tally and PingOutcome::exit_code() applies the table above. dispatch_command returns Result<i32> (every other arm returns 0), and main::dispatch_and_exit is the single place that turns a nonzero command-level code into the process exit status, rather than scattering std::process::exit calls across command implementations. The pre-existing exec path still exits from src/commands/exec.rs after applying its own ExitCodeStrategy; that is untouched.

main::map_hard_failure handles the other half: a hard Err on the ping path, whether raised by initialize_app (config load failure, no host resolved) or by the executor before any per-host tally exists, prints the chain and exits 255 instead of collapsing into the 1 that means "some hosts answered and some did not". The mapping is scoped to Some(Commands::Ping); every other subcommand keeps the default, where returning Err from main exits 1. A test pins that scoping. Command-line usage errors are still rejected by clap with its own exit code 2, unchanged and identical across subcommands; that is stated in both the man page and the architecture doc rather than silently assumed.

What changed

• src/commands/ping.rs: added PING_SSH_LEVEL_FAILURE (255), PingOutcome, PingOutcome::from_results, and PingOutcome::exit_code; ping_nodes now returns the tally. The two local counters are gone, and the summary line is fed from the same tally so display and exit code cannot disagree.
• src/app/dispatcher.rs: dispatch_command returns Result<i32>; the Some(Commands::Ping) arm translates the outcome, all other arms return EXIT_SUCCESS.
• src/main.rs: added dispatch_and_exit and map_hard_failure; both run_bssh_mode and run_pdsh_mode route through them.
• src/cli/bssh.rs: the Ping long_about now lists 0/1/255 and drops the false "response times" claim (the implementation prints no timing at all). Per the decision on the issue, the text is corrected rather than timing implemented.
• docs/man/bssh.1: .SH EXIT STATUS gains a ping Subcommand subsection so the man page and --help agree.
• docs/architecture/exit-code-strategy.md: new "The ping Contract" section covering the table, the rationale for 255, the implementation, and the error propagation rule; the file structure listing now includes ping.rs and dispatcher.rs.
• ARCHITECTURE.md: the Exit Code Strategy summary and the Exit Codes list record the ping exception and 255.
• CHANGELOG.md: a Fixed entry for the user-visible behavior change (previously always 0, all-unreachable now 255) and a Changed entry for the ping_nodes signature, which is a source break for library consumers.

Tests

• src/commands/ping.rs unit tests (5): exit code 0, 1, 255, and the empty host list, plus a parity test asserting PingOutcome::exit_code() equals ExitCodeStrategy::RequireAllSuccess.calculate() for every case where at least one host answered, so the two cannot drift apart.
• tests/ping_exit_code_test.rs (8, new), following the tests/exit_code_integration_test.rs precedent: four contract tests over PingOutcome, which is the exact value main hands to std::process::exit, covering 0, 1, 255, and the empty list; plus four process-level tests that run the real binary and assert the observed status: all hosts unreachable exits 255, a config file that cannot be loaded exits 255, a host list that resolves to nothing exits 255, and a non-ping subcommand on the same pre-connection failure still exits 1.

Scope note on the process-level layer: the all-success (0) and partial-failure (1) cases need a host that actually accepts an SSH connection, which no offline test can provide, so those two are asserted at the PingOutcome boundary rather than by spawning the binary. The three cases that are deterministic offline are asserted end to end against the real process.

Test plan

• cargo test --lib commands::ping (5 passed)
• cargo test --test ping_exit_code_test (8 passed)
• cargo test --test exit_code_integration_test (17 passed, unchanged)
• cargo test --bin bssh (51 passed), cargo test --test pdsh_compat_test (35 passed)
• cargo clippy --lib --bins --tests -- -D warnings clean, cargo fmt --all
• Manual: bssh -H unreachable-host ping; echo $? prints 255 (was 0)
• Manual: bssh --config /nonexistent-dir/config.yaml -H h1 ping exits 255, ... -H h1 --filter no-such ping exits 255, and the same config failure with an exec command still exits 1
• man ./docs/man/bssh.1 renders the new EXIT STATUS subsection correctly

Closes #245

[inureyes]

Implementation Review Summary

Intent

Make bssh ping report a real exit code (the signed-off 0/1/255 contract from #245) instead of always exiting 0, and correct the long_about that promised exit codes and "response times" the implementation never produced.

Contract verification against the real process path

All four rows were exercised end to end against the built binary, not just at the PingOutcome boundary. The 0 and 1 rows (which the PR body notes cannot be covered by an offline test) were verified manually against a live local sshd with --strict-host-key-checking no:

Scenario	Command	Observed
All hosts connected	-H user@localhost ping	0
One reachable, one failed	-H user@localhost,user@x.invalid ping	1
No host succeeded	-H user@x.invalid ping	255
Config load failure	--config /nonexistent/config.yaml ... ping	255
No host resolved (filter)	-H ... --filter no-such ping	255
No host resolved (empty cluster)	--cluster empty ping	255

The value asserted at the PingOutcome boundary is genuinely the value main passes to the process: dispatch_command returns Ok(outcome.exit_code()), and dispatch_and_exit maps Ok(0) to a normal return and Ok(n) to std::process::exit(n).

The collapse risk called out in the issue is genuinely avoided. executor.execute(...)? inside ping_nodes propagates as Err through dispatch_command to dispatch_and_exit's Err arm and into map_hard_failure, which exits 255. There is no path by which a hard Err reaches the 1 that means "some hosts answered and some did not", because 1 is only ever produced by PingOutcome::exit_code() when succeeded > 0.

Scoping and regression checks

• map_hard_failure is gated on matches!(command, Some(Commands::Ping)). Verified unchanged for every other path: upload / download connection failure exits 1, upload on the same unloadable config exits 1, list exits 0, exec success exits 0, exec forwarding a remote status still exits 42 for exit 42 and 255 for exit 255.
• Error output is byte-identical between the ping path and other subcommands, for both a single-level error and a multi-level Caused by: chain. eprintln!("Error: {error:?}") reproduces Termination exactly.
• dispatch_command's new Result<i32>: every non-ping arm returns EXIT_SUCCESS, CacheStats is unreachable! as before, and the exec arm still delegates its own ExitCodeStrategy exit to src/commands/exec.rs. No arm can return an accidental nonzero.
• pdsh mode is unaffected. PdshCli::to_bssh_cli sets command: None, so the ping arm is unreachable from run_pdsh_mode and map_hard_failure is a no-op there. pdsh_compat_test passes (35).
• Exit code range: PingOutcome::exit_code() can only yield 0, 1, or 255, and every other arm yields 0. Nothing can escape 0-255 or truncate.
• Ctrl+C still exits 130 from inside the executor, unchanged.
• The RequireAllSuccess parity test is real and constrains drift: it compares PingOutcome::from_results(...).exit_code() against ExitCodeStrategy::RequireAllSuccess.calculate(...) over the same results, so a change on either side breaks it.
• No orphaned code. PING_SSH_LEVEL_FAILURE, PingOutcome::from_results, PingOutcome::exit_code, EXIT_SUCCESS, dispatch_and_exit, and map_hard_failure are all reachable from the binary.

Findings Addressed

• Docs claimed exit code 255 is unique to ping (MEDIUM). ARCHITECTURE.md said "ping only" directly above a bullet that says other codes are preserved from the main rank, and exit-code-strategy.md said 255 is a value "no other path produces". Both are false: the default MainRank strategy forwards a remote status verbatim, so bssh -H host "exit 255" exits 255 today, which docs/man/bssh.1 already documents as 1-255 Main rank failed with this exit code. Reworded in 89fd812 to say ping is the only path that generates 255 as a bssh-level signal.

Remaining Items

• PingOutcome::total is populated but never read outside tests, and ping_nodes still passes nodes.len() (not outcome.total) as the first argument to format_summary (LOW). Harmless today, since ping never enables fail-fast so results.len() == nodes.len(), but it makes the PR body's "the summary line is fed from the same tally" true only for the succeeded and failed columns. Left as is.
• run_pdsh_mode still uses a bare ? on initialize_app rather than routing through map_hard_failure (informational). Correct today because pdsh always sets command: None, so the mapping would be a no-op.

Documentation accuracy

--help, docs/man/bssh.1 .SH EXIT STATUS, docs/architecture/exit-code-strategy.md, ARCHITECTURE.md, and CHANGELOG.md now agree with each other and with the code. The clap exit code 2 deviation is documented accurately: verified that list, ping, upload, download, and interactive all exit 2 on a usage error, and that a missing required argument also exits 2. No stale "response times" or "0 (all reachable), 1 (any unreachable)" text remains anywhere in the repo.

Verification

• All stated requirements implemented
• No placeholder/mock code remaining
• Integrated into project code flow
• Project conventions followed
• Existing modules reused where applicable (ExitCodeStrategy::RequireAllSuccess, pinned by a parity test)
• No unintended structural changes
• Tests pass (--lib commands::ping 5, --test ping_exit_code_test 8, --test exit_code_integration_test 17, --bin bssh 51, --test pdsh_compat_test 35; cargo clippy --lib --bins --tests -- -D warnings and cargo fmt --check clean)