Skip to content

Commit 8fba0d1

Browse files
UnbreakableMJclaude
andcommitted
feat(config+session): M18.2 — plumb ConnectTimeout/ConnectionAttempts; wrap connect with retry (FR-80)
Closes the M12.6 ConnectTimeout / ConnectionAttempts deferral loop. The primary `AnvilSession::connect` path now retries transient TCP failures with jittered exponential backoff per the M18.1 retry module; per-attempt connect timeouts wrap russh's `client::connect` in `tokio::time::timeout` when configured. src/config.rs: - AnvilConfig: three new public fields: connect_timeout: Option<Duration> connection_attempts: Option<u32> max_retry_window: Option<Duration> Each None falls through to retry::RetryPolicy::default() at session-build time. - AnvilConfigBuilder: matching three setters. CLI applies them AFTER apply_ssh_config so flags beat config (matches OpenSSH precedence). - apply_ssh_config: now consumes resolved.connect_timeout and resolved.connection_attempts, but only if the builder field is still None (preserves CLI-wins precedence). max_retry_window is CLI-only — not in OpenSSH's grammar. - warn_unhonored_directives helper REMOVED. Every ssh_config(5) directive Anvil's resolver parses today is now consumed: HostKeyAlgorithms / KexAlgorithms / Ciphers / MACs landed in M17; ConnectTimeout / ConnectionAttempts in M18. src/session.rs: - AnvilSession: new private field `retry_history: Vec<RetryAttempt>` capturing per-attempt failures during the connect path. Surfaced via the new pub fn AnvilSession::retry_history(&self) -> &[RetryAttempt] accessor for `gitway --test --json`'s data.retry_attempts envelope (FR-83). Empty when first attempt succeeded. - connect(): wrapped in retry::run. Each attempt: 1. Build fresh HandlerPieces (russh consumes the handler). 2. Call client::connect(...), wrapped in tokio::time::timeout when policy.connect_timeout is Some. Elapsed → mapped to AnvilErrorKind::Io(io::ErrorKind::TimedOut) which the FR-82 classifier treats as Retry. 3. Return (handle, ConnectArtifacts). Retry history flows from retry::run into the AnvilSession field. Auth / host-key / protocol errors are fatal per FR-82 and surface immediately without retry. - New private struct ConnectArtifacts and helper retry_policy_from_config(config) extracted to keep the closure signature concise. - connect_via_proxy_command and connect_via_jump_hosts (final hop + per-hop) construct AnvilSession with retry_history: Vec::new() for now. The plan PR body documents this scope-narrowing: the ProxyCommand subprocess lifecycle and per-hop direct-tcpip channels make retry semantics murkier than the primary path; deferred to a follow-up sub-milestone. FR-80..FR-83 are met for `gitway --test` against a direct target host today; the proxy / jump paths fall back to the pre-M18 single-attempt behaviour and surface the same opaque IO error they did before. Public API: pure additive (three new pub fields on AnvilConfig + three new builder setters + new retry_history accessor). build_russh_config signature unchanged. 207 lib tests + integration tests still pass. Plan: M18.2 of anvil-gitway-milestone-plan.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent b143f5e commit 8fba0d1

2 files changed

Lines changed: 184 additions & 39 deletions

File tree

src/config.rs

Lines changed: 76 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,19 @@ pub struct AnvilConfig {
108108
/// Host-key algorithm preference (PRD §5.8.6 FR-76). `None` →
109109
/// curated default.
110110
pub host_key_algorithms: Option<Vec<String>>,
111+
/// Per-attempt TCP connect timeout (PRD §5.8.7 FR-80). `None`
112+
/// disables the timeout (matches OpenSSH's "no `ConnectTimeout`"
113+
/// semantics).
114+
pub connect_timeout: Option<Duration>,
115+
/// Total number of connection attempts including the initial one
116+
/// (PRD §5.8.7 FR-80). `None` selects the curated default
117+
/// ([`crate::retry::RetryPolicy`]'s `attempts = 3`).
118+
pub connection_attempts: Option<u32>,
119+
/// Hard ceiling on total elapsed wall-clock time across all
120+
/// retry attempts (PRD §5.8.7 FR-81). `None` selects the
121+
/// curated default (30 s). CLI-only — not part of OpenSSH's
122+
/// `ssh_config(5)` grammar.
123+
pub max_retry_window: Option<Duration>,
111124
}
112125

113126
impl AnvilConfig {
@@ -191,6 +204,9 @@ pub struct AnvilConfigBuilder {
191204
ciphers: Option<Vec<String>>,
192205
macs: Option<Vec<String>>,
193206
host_key_algorithms: Option<Vec<String>>,
207+
connect_timeout: Option<Duration>,
208+
connection_attempts: Option<u32>,
209+
max_retry_window: Option<Duration>,
194210
}
195211

196212
impl AnvilConfigBuilder {
@@ -219,6 +235,15 @@ impl AnvilConfigBuilder {
219235
ciphers: None,
220236
macs: None,
221237
host_key_algorithms: None,
238+
// Retry / timeout knobs default to None. At session
239+
// build time, None falls through to
240+
// `crate::retry::RetryPolicy::default()` (3 attempts,
241+
// 250 ms base, 30 s max_window, no connect timeout) —
242+
// matches OpenSSH's defaults except for the new
243+
// max_window cap which is Gitway-specific.
244+
connect_timeout: None,
245+
connection_attempts: None,
246+
max_retry_window: None,
222247
}
223248
}
224249

@@ -351,6 +376,30 @@ impl AnvilConfigBuilder {
351376
self
352377
}
353378

379+
/// Override the per-attempt TCP connect timeout (PRD §5.8.7
380+
/// FR-80). `None` disables the timeout. CLI overrides this
381+
/// AFTER `apply_ssh_config` so flags beat config (matches
382+
/// OpenSSH precedence).
383+
pub fn connect_timeout(mut self, timeout: Option<Duration>) -> Self {
384+
self.connect_timeout = timeout;
385+
self
386+
}
387+
388+
/// Override the total connection-attempt count (PRD §5.8.7
389+
/// FR-80). `None` selects the curated default (3).
390+
pub fn connection_attempts(mut self, attempts: Option<u32>) -> Self {
391+
self.connection_attempts = attempts;
392+
self
393+
}
394+
395+
/// Override the wall-clock cap on total retry time (PRD
396+
/// §5.8.7 FR-81). `None` selects the curated default (30 s).
397+
/// Not part of OpenSSH's `ssh_config(5)` grammar — CLI-only.
398+
pub fn max_retry_window(mut self, window: Option<Duration>) -> Self {
399+
self.max_retry_window = window;
400+
self
401+
}
402+
354403
/// Layer values from a [`ResolvedSshConfig`] into this builder.
355404
///
356405
/// Provides ssh_config-derived defaults that subsequent builder calls
@@ -432,7 +481,23 @@ impl AnvilConfigBuilder {
432481
crate::algorithms::anvil_default_host_keys,
433482
|b, v| b.host_key_algorithms = Some(v),
434483
);
435-
warn_unhonored_directives(resolved);
484+
485+
// M18 / FR-80: ConnectTimeout + ConnectionAttempts from
486+
// ssh_config flow through to the retry-policy fields. CLI
487+
// overrides applied AFTER apply_ssh_config win over these
488+
// (matches OpenSSH precedence). Don't clobber a value
489+
// already set on the builder — that's how the CLI-wins
490+
// precedence is achieved.
491+
if self.connect_timeout.is_none() {
492+
if let Some(d) = resolved.connect_timeout {
493+
self.connect_timeout = Some(d);
494+
}
495+
}
496+
if self.connection_attempts.is_none() {
497+
if let Some(n) = resolved.connection_attempts {
498+
self.connection_attempts = Some(n);
499+
}
500+
}
436501
self
437502
}
438503

@@ -484,40 +549,20 @@ impl AnvilConfigBuilder {
484549
ciphers: self.ciphers,
485550
macs: self.macs,
486551
host_key_algorithms: self.host_key_algorithms,
552+
connect_timeout: self.connect_timeout,
553+
connection_attempts: self.connection_attempts,
554+
max_retry_window: self.max_retry_window,
487555
}
488556
}
489557
}
490558

491-
/// Emits a single `log::warn!` listing any directives in `resolved` that
492-
/// the resolver successfully parsed but Anvil does not yet honor. Called
493-
/// from [`AnvilConfigBuilder::apply_ssh_config`] so the warning fires
494-
/// when a config is actually being prepared for connection — `gitway
495-
/// config show` and similar inspection callers do not trigger it.
496-
///
497-
/// The remaining unhonored set after M17:
498-
/// - `ConnectTimeout`, `ConnectionAttempts` — parsed but not yet wired
499-
/// into `connect()`. M18 honors them.
500-
///
501-
/// `HostKeyAlgorithms` / `KexAlgorithms` / `Ciphers` / `MACs` were
502-
/// the M17 deferral; they're now consumed by `apply_ssh_config` via
503-
/// the `apply_alg_directive` helper above.
504-
fn warn_unhonored_directives(resolved: &ResolvedSshConfig) {
505-
let mut m18: Vec<&'static str> = Vec::new();
506-
if resolved.connect_timeout.is_some() {
507-
m18.push("ConnectTimeout");
508-
}
509-
if resolved.connection_attempts.is_some() {
510-
m18.push("ConnectionAttempts");
511-
}
512-
513-
if !m18.is_empty() {
514-
log::warn!(
515-
"ssh_config: directive(s) {} parsed but not yet honored \
516-
(landing in M18 — Gitway PRD §8)",
517-
m18.join(", "),
518-
);
519-
}
520-
}
559+
// Note: the `warn_unhonored_directives` helper that lived here from
560+
// M12.6 → M17.2 → M18.2 was removed in M18.2. Every `ssh_config(5)`
561+
// directive Anvil's resolver parses today is now consumed by
562+
// `apply_ssh_config` (HostKeyAlgorithms / KexAlgorithms / Ciphers /
563+
// MACs landed in M17; ConnectTimeout / ConnectionAttempts in M18).
564+
// Future deferral warnings should reintroduce a similar helper if a
565+
// new milestone parses-but-doesn't-yet-consume a directive.
521566

522567
#[cfg(test)]
523568
mod tests {

src/session.rs

Lines changed: 108 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,13 @@ pub struct AnvilSession {
242242
auth_banner: Arc<Mutex<Option<String>>>,
243243
/// SHA-256 fingerprint of the server key that passed verification, if any.
244244
verified_fingerprint: Arc<Mutex<Option<String>>>,
245+
/// Per-attempt history captured by [`crate::retry::run`] during
246+
/// the connect path (M18, FR-83). Empty when the first attempt
247+
/// succeeded; otherwise one entry per failed attempt that
248+
/// triggered a retry, plus the final attempt that succeeded.
249+
/// Surfaced via [`Self::retry_history`] for the
250+
/// `gitway --test --json` envelope.
251+
retry_history: Vec<crate::retry::RetryAttempt>,
245252
}
246253

247254
/// Manual Debug impl because `client::Handle<H>` does not implement `Debug`.
@@ -345,20 +352,55 @@ impl AnvilSession {
345352
/// Does **not** authenticate; call [`authenticate`](Self::authenticate) or
346353
/// [`authenticate_best`](Self::authenticate_best) after this.
347354
///
355+
/// **M18 / FR-80 / FR-81 / FR-82**: the TCP connect is wrapped in
356+
/// a [`crate::retry::run`] loop with per-attempt
357+
/// [`tokio::time::timeout`] (when `config.connect_timeout` is
358+
/// `Some`). Transient failures (`ECONNREFUSED`, `ETIMEDOUT`,
359+
/// DNS NXDOMAIN, …) are retried with jittered exponential
360+
/// backoff; auth / host-key / protocol errors are fatal and
361+
/// surface immediately. See [`Self::retry_history`] for the
362+
/// per-attempt history captured during the loop.
363+
///
348364
/// # Errors
349365
///
350-
/// Returns an error on network failure or if the server's host key does not
351-
/// match any pinned fingerprint.
366+
/// Returns an error on terminal network failure (after exhausting
367+
/// the retry budget) or if the server's host key does not
368+
/// match any pinned fingerprint (fatal — never retried).
352369
pub async fn connect(config: &AnvilConfig) -> Result<Self, AnvilError> {
353-
let pieces = Self::build_handler_pieces(config)?;
370+
let policy = retry_policy_from_config(config);
354371

355372
log::debug!("session: connecting to {}:{}", config.host, config.port);
356373

357-
let handle = client::connect(
358-
pieces.russh_cfg,
359-
(config.host.as_str(), config.port),
360-
pieces.handler,
361-
)
374+
// Each attempt rebuilds `pieces` because russh's
375+
// `client::connect` consumes the handler. This is cheap —
376+
// `build_handler_pieces` is pure setup, no I/O.
377+
let ((handle, pieces), retry_history) = crate::retry::run(&policy, || async {
378+
let pieces = Self::build_handler_pieces(config)?;
379+
let connect_fut = client::connect(
380+
pieces.russh_cfg,
381+
(config.host.as_str(), config.port),
382+
pieces.handler,
383+
);
384+
let handle = match policy.connect_timeout {
385+
Some(t) => match tokio::time::timeout(t, connect_fut).await {
386+
Ok(Ok(h)) => h,
387+
Ok(Err(e)) => return Err(e),
388+
Err(_elapsed) => {
389+
return Err(AnvilError::new(crate::error::AnvilErrorKind::Io(
390+
std::io::Error::from(std::io::ErrorKind::TimedOut),
391+
)));
392+
}
393+
},
394+
None => connect_fut.await?,
395+
};
396+
Ok((
397+
handle,
398+
ConnectArtifacts {
399+
auth_banner: pieces.auth_banner,
400+
verified_fingerprint: pieces.verified_fingerprint,
401+
},
402+
))
403+
})
362404
.await?;
363405

364406
log::debug!("session: SSH handshake complete with {}", config.host);
@@ -367,9 +409,23 @@ impl AnvilSession {
367409
handle,
368410
auth_banner: pieces.auth_banner,
369411
verified_fingerprint: pieces.verified_fingerprint,
412+
retry_history,
370413
})
371414
}
372415

416+
/// Returns the [`crate::retry::RetryAttempt`] history captured
417+
/// during the most-recent `connect*` call on this session.
418+
///
419+
/// Empty when the first attempt succeeded. Non-empty when the
420+
/// retry loop fired at least once; the last entry's `attempt`
421+
/// matches the attempt number that ultimately succeeded.
422+
/// Surfaced by `gitway --test --json`'s `data.retry_attempts`
423+
/// envelope (FR-83).
424+
#[must_use]
425+
pub fn retry_history(&self) -> &[crate::retry::RetryAttempt] {
426+
&self.retry_history
427+
}
428+
373429
/// Establishes the SSH session through a chain of `ProxyJump`
374430
/// bastion hops (FR-56).
375431
///
@@ -508,6 +564,11 @@ impl AnvilSession {
508564
handle,
509565
auth_banner: pieces.auth_banner,
510566
verified_fingerprint: pieces.verified_fingerprint,
567+
// Per-hop retry not yet wired through M18.2 — see
568+
// M18.2 commit body. Empty history; consumers
569+
// reading retry_history() see only the primary
570+
// connect's attempts.
571+
retry_history: Vec::new(),
511572
};
512573
hop_session
513574
.authenticate_best(&hop_config)
@@ -563,6 +624,12 @@ impl AnvilSession {
563624
handle: final_handle,
564625
auth_banner: target_pieces.auth_banner,
565626
verified_fingerprint: target_pieces.verified_fingerprint,
627+
// M18.2: ProxyJump chain doesn't yet have retry wired
628+
// for the per-hop connects (each hop's
629+
// direct-tcpip channel makes the retry semantics
630+
// murkier). Documented as scoped-out in the M18.2
631+
// commit body; deferred to a follow-up.
632+
retry_history: Vec::new(),
566633
})
567634
}
568635

@@ -629,6 +696,11 @@ impl AnvilSession {
629696
handle,
630697
auth_banner: pieces.auth_banner,
631698
verified_fingerprint: pieces.verified_fingerprint,
699+
// M18.2: ProxyCommand connect path doesn't yet have
700+
// retry wired (the subprocess lifecycle complicates
701+
// re-spawning per attempt). Documented as scoped-out
702+
// in the M18.2 commit body; deferred to a follow-up.
703+
retry_history: Vec::new(),
632704
})
633705
}
634706

@@ -1021,6 +1093,34 @@ impl AnvilSession {
10211093
/// constant for) are silently dropped here because russh's `Name`
10221094
/// types only accept `&'static str`; a future v1.1 may surface
10231095
/// these via a hard error at the override-validation stage.
1096+
/// Per-attempt artifacts captured during a successful connect inside
1097+
/// [`AnvilSession::connect`]'s retry loop. Keeps the closure
1098+
/// signature concise and avoids leaking `HandlerPieces`'s internals
1099+
/// (e.g. the consumed-on-connect `russh_cfg` Arc) past the loop
1100+
/// boundary.
1101+
struct ConnectArtifacts {
1102+
auth_banner: Arc<Mutex<Option<String>>>,
1103+
verified_fingerprint: Arc<Mutex<Option<String>>>,
1104+
}
1105+
1106+
/// Builds a [`crate::retry::RetryPolicy`] from the connect-related
1107+
/// fields on [`AnvilConfig`] (M18, FR-80 / FR-81). Each `None`
1108+
/// field falls through to the corresponding `RetryPolicy::default()`
1109+
/// value.
1110+
fn retry_policy_from_config(config: &AnvilConfig) -> crate::retry::RetryPolicy {
1111+
let mut policy = crate::retry::RetryPolicy::default();
1112+
if let Some(t) = config.connect_timeout {
1113+
policy.connect_timeout = Some(t);
1114+
}
1115+
if let Some(n) = config.connection_attempts {
1116+
policy.attempts = n.max(1);
1117+
}
1118+
if let Some(w) = config.max_retry_window {
1119+
policy.max_window = w;
1120+
}
1121+
policy
1122+
}
1123+
10241124
fn build_russh_config(config: &AnvilConfig) -> client::Config {
10251125
let kex_strings = config
10261126
.kex_algorithms

0 commit comments

Comments
 (0)