From 9a83b2dcb46135da0a02a59114acf6f399ff365d Mon Sep 17 00:00:00 2001 From: Anuoluwapo25 Date: Wed, 27 May 2026 22:40:23 +0100 Subject: [PATCH] feat: remote preflight, protocol version exposure, retry validation, timestamp helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements four issues across the remote debugging and output subsystems: #1254 – Remote: add preflight subcommand - `soroban-debug remote preflight [--output json|pretty]` checks TCP connect, protocol handshake, auth, optional TLS, and an end-to-end ping without loading any contract. - Both pretty and JSON output modes are supported; actionable suggestions are embedded in failure entries. #1255 – Remote: expose selected protocol version in inspect output - `RemoteClient::selected_protocol_version()` getter added. - Protocol version is now printed in verbose `remote` output and in the `remote inspect` output when `--verbose` is active. - `doctor --remote` now populates `selected_protocol` in its JSON report and prints it in the human-readable section. #1257 – Remote: validate retry configuration before connecting - `--retry-attempts 0` is now rejected with a clear error and suggestion. - `--retry-max-delay-ms` less than `--retry-base-delay-ms` is rejected with values and a correction hint. - Help text documents the constraints and environment variable interaction. - Man page updated automatically from the arg docstrings. #1252 – Output: add stable timestamp formatting helpers - `format_timestamp_iso8601(SystemTime) -> String` and `now_iso8601()` added to `src/output.rs` using the existing `chrono` dependency. - Output is always UTC with a `Z` suffix; no locale dependency. - Three unit tests cover the epoch, a known calendar date, and the structural format of the current-time helper. --- man/man1/soroban-debug-remote.1 | 9 ++ src/cli/args.rs | 17 +++ src/cli/commands.rs | 182 +++++++++++++++++++++++++++++++- src/client/remote_client.rs | 5 + src/output.rs | 43 ++++++++ 5 files changed, 252 insertions(+), 4 deletions(-) diff --git a/man/man1/soroban-debug-remote.1 b/man/man1/soroban-debug-remote.1 index bb34d5e9..a211d2d6 100644 --- a/man/man1/soroban-debug-remote.1 +++ b/man/man1/soroban-debug-remote.1 @@ -73,16 +73,22 @@ May also be specified with the \fBSOROBAN_DEBUG_STORAGE_TIMEOUT_MS\fR environmen \fB\-\-retry\-attempts\fR \fI\fR [default: 3] Maximum number of retry attempts for idempotent requests (ping, inspect, storage). +Must be at least 1. Use 1 to disable retries entirely (one attempt, no retry). + Default: 3. .TP \fB\-\-retry\-base\-delay\-ms\fR \fI\fR [default: 200] Base delay in milliseconds between retry attempts (exponential back\-off). +Must be greater than 0 and no larger than \-\-retry\-max\-delay\-ms. + Default: 200 ms. .TP \fB\-\-retry\-max\-delay\-ms\fR \fI\fR [default: 2000] Maximum delay in milliseconds between retry attempts. +Must be greater than or equal to \-\-retry\-base\-delay\-ms. + Default: 2 000 ms. .TP \fB\-h\fR, \fB\-\-help\fR @@ -98,5 +104,8 @@ Get contract storage state as JSON remote\-evaluate(1) Evaluate an expression in the current debug context .TP +remote\-preflight(1) +Run a preflight check: connect, handshake, auth, and optional TLS validation without loading a contract. Suitable for CI health checks and troubleshooting +.TP remote\-help(1) Print this message or the help of the given subcommand(s) diff --git a/src/cli/args.rs b/src/cli/args.rs index ab4e037e..e3404ca8 100644 --- a/src/cli/args.rs +++ b/src/cli/args.rs @@ -1299,18 +1299,24 @@ pub struct RemoteArgs { /// Maximum number of retry attempts for idempotent requests (ping, inspect, storage). /// + /// Must be at least 1. Use 1 to disable retries entirely (one attempt, no retry). + /// /// Default: 3. #[arg(long, value_name = "N", default_value = "3")] pub retry_attempts: usize, /// Base delay in milliseconds between retry attempts (exponential back-off). /// + /// Must be greater than 0 and no larger than --retry-max-delay-ms. + /// /// Default: 200 ms. #[arg(long, value_name = "MS", default_value = "200")] pub retry_base_delay_ms: u64, /// Maximum delay in milliseconds between retry attempts. /// + /// Must be greater than or equal to --retry-base-delay-ms. + /// /// Default: 2 000 ms. #[arg(long, value_name = "MS", default_value = "2000")] pub retry_max_delay_ms: u64, @@ -1330,6 +1336,17 @@ pub enum RemoteAction { /// Evaluate an expression in the current debug context Evaluate(RemoteEvaluateArgs), + + /// Run a preflight check: connect, handshake, auth, and optional TLS validation + /// without loading a contract. Suitable for CI health checks and troubleshooting. + Preflight(PreflightArgs), +} + +#[derive(Parser)] +pub struct PreflightArgs { + /// Output format (pretty or json) + #[arg(long = "output", value_enum, default_value_t = OutputFormat::Pretty)] + pub output_format: OutputFormat, } #[derive(Parser)] diff --git a/src/cli/commands.rs b/src/cli/commands.rs index 23c4a1a9..0eb0b65e 100644 --- a/src/cli/commands.rs +++ b/src/cli/commands.rs @@ -6,8 +6,9 @@ use crate::analyzer::{ }; use crate::cli::args::{ AnalyzeArgs, CompareArgs, HistoryPruneArgs, InspectArgs, InteractiveArgs, OptimizeArgs, - OutputFormat, ProfileArgs, RemoteAction, RemoteArgs, ReplArgs, ReplayArgs, RunArgs, - ScenarioArgs, ServerArgs, SymbolicArgs, SymbolicProfile, TuiArgs, UpgradeCheckArgs, Verbosity, + OutputFormat, PreflightArgs, ProfileArgs, RemoteAction, RemoteArgs, ReplArgs, ReplayArgs, + RunArgs, ScenarioArgs, ServerArgs, SymbolicArgs, SymbolicProfile, TuiArgs, UpgradeCheckArgs, + Verbosity, }; use crate::cli::output::write_json_pretty_file; use crate::debugger::engine::DebuggerEngine; @@ -2037,7 +2038,33 @@ pub fn server(args: ServerArgs) -> Result<()> { } /// Connect to remote debug server -pub fn remote(args: RemoteArgs, _verbosity: Verbosity) -> Result<()> { +pub fn remote(args: RemoteArgs, verbosity: Verbosity) -> Result<()> { + // #1257 — validate retry configuration before attempting any connection. + if args.retry_attempts == 0 { + return Err(miette::miette!( + "Invalid --retry-attempts value: 0 is not allowed.\n\ + \n\ + At least one attempt is required for the remote command to make any request.\n\ + Use --retry-attempts 1 to disable retries (single attempt, no retry on failure).\n\ + \n\ + Tip: the default is 3 attempts with exponential back-off." + )); + } + if args.retry_max_delay_ms < args.retry_base_delay_ms { + return Err(miette::miette!( + "Invalid retry delay configuration: --retry-max-delay-ms ({max}ms) is less than \ + --retry-base-delay-ms ({base}ms).\n\ + \n\ + The maximum back-off delay must be greater than or equal to the base delay.\n\ + Suggestion: set --retry-max-delay-ms to at least {base}ms, or reduce \ + --retry-base-delay-ms.\n\ + \n\ + Defaults: --retry-base-delay-ms 200 --retry-max-delay-ms 2000", + base = args.retry_base_delay_ms, + max = args.retry_max_delay_ms, + )); + } + print_info(format!("Connecting to remote debugger at {}", args.remote)); // Build per-request timeouts, falling back to the general --timeout-ms for @@ -2078,6 +2105,11 @@ pub fn remote(args: RemoteArgs, _verbosity: Verbosity) -> Result<()> { } })?; + // #1255 — show the negotiated protocol version in verbose output. + if let Some(proto) = client.selected_protocol_version() { + print_verbose(format!("Protocol version negotiated: {}", proto)); + } + if let Some(info) = client.session_info() { print_info(format!( "Remote session: {} (created {}, label={})", @@ -2103,6 +2135,12 @@ pub fn remote(args: RemoteArgs, _verbosity: Verbosity) -> Result<()> { if let Some(reason) = pause_reason { println!("Pause reason: {}", reason); } + // #1255 — include protocol in verbose inspect output. + if verbosity == Verbosity::Verbose { + if let Some(proto) = client.selected_protocol_version() { + println!("Protocol version: {}", proto); + } + } if !call_stack.is_empty() { println!("Call stack:"); for frame in &call_stack { @@ -2126,6 +2164,9 @@ pub fn remote(args: RemoteArgs, _verbosity: Verbosity) -> Result<()> { } Ok(()) } + RemoteAction::Preflight(preflight_args) => { + run_remote_preflight(&args, preflight_args, client) + } }; } @@ -2140,6 +2181,135 @@ pub fn remote(args: RemoteArgs, _verbosity: Verbosity) -> Result<()> { print_success("Remote debugger is reachable"); Ok(()) } +/// Output produced by the `remote preflight` subcommand. +#[derive(Debug, serde::Serialize)] +struct PreflightReport { + address: String, + connect: PreflightCheck, + handshake: PreflightCheck, + protocol_version: Option, + auth: Option, + tls: Option, + ping: PreflightCheck, + overall: bool, +} + +#[derive(Debug, serde::Serialize)] +struct PreflightCheck { + ok: bool, + message: String, + #[serde(skip_serializing_if = "Option::is_none")] + suggestion: Option, +} + +impl PreflightCheck { + fn ok(message: impl Into) -> Self { + Self { ok: true, message: message.into(), suggestion: None } + } + fn err(message: impl Into, suggestion: impl Into) -> Self { + Self { + ok: false, + message: message.into(), + suggestion: Some(suggestion.into()), + } + } +} + +/// Implements `remote preflight` (#1254): checks connect, handshake, auth, and TLS +/// without loading any contract. +fn run_remote_preflight( + args: &RemoteArgs, + preflight_args: &PreflightArgs, + mut client: crate::client::RemoteClient, +) -> Result<()> { + // The client was already successfully connected and the handshake completed — + // `connect_with_config` performs handshake and auth internally, so we just + // need to record the outcomes and then ping to confirm the channel is live. + let connect_check = PreflightCheck::ok(format!("TCP connection established to {}", args.remote)); + + let handshake_check = PreflightCheck::ok("Protocol handshake succeeded"); + let protocol_version = client.selected_protocol_version(); + + let auth_check = if args.token.is_some() { + Some(PreflightCheck::ok("Authentication succeeded")) + } else { + None + }; + + let tls_check = if args.tls_cert.is_some() || args.tls_key.is_some() || args.tls_ca.is_some() { + Some(PreflightCheck::ok("TLS handshake succeeded")) + } else { + None + }; + + // A final ping confirms end-to-end liveness. + let ping_check = match client.ping() { + Ok(_) => PreflightCheck::ok("End-to-end ping succeeded"), + Err(e) => PreflightCheck::err( + format!("End-to-end ping failed: {}", e), + "Verify the server process is running and the port is reachable. \ + Use --connect-timeout-ms to extend the TCP window if the server is slow to respond.", + ), + }; + let overall = ping_check.ok; + + let report = PreflightReport { + address: args.remote.clone(), + connect: connect_check, + handshake: handshake_check, + protocol_version, + auth: auth_check, + tls: tls_check, + ping: ping_check, + overall, + }; + + if preflight_args.output_format == OutputFormat::Json { + let json = serde_json::to_string_pretty(&report) + .map_err(|e| miette::miette!("Failed to serialize preflight report: {}", e))?; + println!("{}", json); + return Ok(()); + } + + // Pretty output + let check = |c: &PreflightCheck| { + if c.ok { + println!(" [PASS] {}", c.message); + } else { + println!(" [FAIL] {}", c.message); + if let Some(ref hint) = c.suggestion { + println!(" Suggestion: {}", hint); + } + } + }; + + println!("Preflight check: {}", report.address); + println!(); + check(&report.connect); + check(&report.handshake); + if let Some(proto) = report.protocol_version { + println!(" [INFO] Negotiated protocol version: {}", proto); + } + if let Some(ref a) = report.auth { + check(a); + } else { + println!(" [SKIP] Auth (no --token provided)"); + } + if let Some(ref t) = report.tls { + check(t); + } else { + println!(" [SKIP] TLS (no TLS flags provided)"); + } + check(&report.ping); + println!(); + if overall { + print_success("Preflight passed."); + } else { + println!("Preflight FAILED. Check the items above for actionable suggestions."); + } + Ok(()) +} + /// Launch interactive debugger UI pub fn interactive(args: InteractiveArgs, _verbosity: Verbosity) -> Result<()> { print_info(format!("Loading contract: {:?}", args.contract)); @@ -2957,6 +3127,7 @@ pub fn doctor(args: crate::cli::args::DoctorArgs) -> Result<()> { config, ) { Ok(mut client) => { + let selected_protocol = client.selected_protocol_version(); let ping = match client.ping() { Ok(_) => Some(check_ok("Ping succeeded")), Err(e) => Some(check_err(format!("Ping failed: {}", e))), @@ -2970,7 +3141,7 @@ pub fn doctor(args: crate::cli::args::DoctorArgs) -> Result<()> { .token .as_ref() .map(|_| check_ok("Authentication succeeded")), - selected_protocol: None, + selected_protocol, }) } Err(e) => Some(RemoteDoctorReport { @@ -3013,6 +3184,9 @@ pub fn doctor(args: crate::cli::args::DoctorArgs) -> Result<()> { if let Some(handshake) = remote.handshake { println!("Remote handshake: {}", handshake.message); } + if let Some(proto) = remote.selected_protocol { + println!("Remote protocol version: {}", proto); + } if let Some(ping) = remote.ping { println!("Remote ping: {}", ping.message); } diff --git a/src/client/remote_client.rs b/src/client/remote_client.rs index bb61ea48..b5d43dc5 100644 --- a/src/client/remote_client.rs +++ b/src/client/remote_client.rs @@ -901,6 +901,11 @@ impl RemoteClient { self.session_id.as_deref() } + /// Returns the protocol version negotiated during the handshake, if any. + pub fn selected_protocol_version(&self) -> Option { + self.selected_protocol_version + } + fn timeout_for_class(&self, class: RequestClass) -> Duration { match class { RequestClass::Ping => self.config.timeouts.ping, diff --git a/src/output.rs b/src/output.rs index ca0fdd10..19a0dfc2 100644 --- a/src/output.rs +++ b/src/output.rs @@ -6,6 +6,20 @@ use serde::{Deserialize, Serialize}; use std::fmt::Write; use std::sync::atomic::{AtomicBool, Ordering}; +/// Format a `SystemTime` as a UTC ISO 8601 string (e.g. `"2026-05-27T12:34:56Z"`). +/// +/// Uses only the standard library — no locale dependency. +pub fn format_timestamp_iso8601(time: std::time::SystemTime) -> String { + use chrono::{DateTime, Utc}; + let dt: DateTime = time.into(); + dt.format("%Y-%m-%dT%H:%M:%SZ").to_string() +} + +/// Return the current UTC time as an ISO 8601 string. +pub fn now_iso8601() -> String { + format_timestamp_iso8601(std::time::SystemTime::now()) +} + static NO_UNICODE: AtomicBool = AtomicBool::new(false); static COLORS_ENABLED: AtomicBool = AtomicBool::new(true); pub const SCHEMA_VERSION: &str = "1.0.0"; @@ -619,6 +633,35 @@ pub fn format_resource_timeline( mod tests { use super::*; + #[test] + fn format_timestamp_iso8601_produces_utc_z_suffix() { + let epoch = std::time::SystemTime::UNIX_EPOCH; + let s = format_timestamp_iso8601(epoch); + assert_eq!(s, "1970-01-01T00:00:00Z"); + } + + #[test] + fn format_timestamp_iso8601_known_value() { + // 2025-05-27 00:00:00 UTC => 1748304000 seconds since epoch + let t = std::time::SystemTime::UNIX_EPOCH + + std::time::Duration::from_secs(1_748_304_000); + let s = format_timestamp_iso8601(t); + assert_eq!(s, "2025-05-27T00:00:00Z"); + } + + #[test] + fn now_iso8601_has_expected_format() { + let s = now_iso8601(); + // Must look like YYYY-MM-DDTHH:MM:SSZ (20 chars) + assert_eq!(s.len(), 20, "unexpected length: {}", s); + assert!(s.ends_with('Z'), "must end with Z: {}", s); + assert_eq!(&s[4..5], "-"); + assert_eq!(&s[7..8], "-"); + assert_eq!(&s[10..11], "T"); + assert_eq!(&s[13..14], ":"); + assert_eq!(&s[16..17], ":"); + } + #[test] fn test_replay_bundle_serializes() { let bundle = SymbolicReplayBundle {