Skip to content

feat(ssh): recover fast after a network change by invalidating stale connections and resetting poll backoff #123

Description

@keinstn

Summary

After a Wi-Fi switch or roaming between networks, drover shows a "cannot reach the host"
state for far longer than the network outage itself lasts. Nothing in the connection stack
resets on a network change, so recovery is paced entirely by timeouts and backoff:

  1. The cached SSHClient is not isClosed (dartssh2 has not noticed the path died), so
    SshCommandRunner._ensureClient() reuses a dead connection
    (app/lib/src/infra/ssh_command_runner.dart:194-209) and the failure only surfaces when
    _execTimeout (15s, :221) fires.
  2. On the herd screen a further herdPollBackoff escalates the wait —
    pollInterval * 2^min(failStreak, 5) capped at 30s
    (app/lib/src/screens/herd_screen.dart:45-49). Even once the network is healthy again,
    drover deliberately waits out that window before retrying.

The host side loses nothing during this: herdr runs a persistent server on the host and
drover talks to it over a socket API per exec call, so agent state survives the disconnect.
What is missing is fast, silent re-attachment.

The fix is to treat an OS-level network change as a signal that cached transport state is
stale: drop the cached SSH clients and clear the failure backoff so the next ordinary poll
reconnects immediately instead of waiting out a timeout plus backoff.

This was considered as an alternative to adopting the mosh protocol. mosh's roaming
property (a session keyed by a secret rather than a TCP 5-tuple) is genuinely the right
shape for this problem, but it was rejected on cost: there is no Dart mosh client, the
only mobile precedent (Blink Shell) cross-compiles the C++ core to a native framework,
UDP 60000-61000 is often blocked, and CommandRunner needs SFTP-equivalent file
operations that mosh has no channel for — which would mean maintaining two transports
with two auth paths permanently. This issue is the cheap approximation of the same
property.

Scope

In:

  • Add a network-change signal source (connectivity_plus, not currently a dependency)
    behind a thin abstraction so it can be faked in tests
  • On a network change, invalidate cached SSH connections so the next call reconnects
    fresh — HostConnectionRegistry (app/lib/src/infra/host_connections.dart) already
    exposes evict(hostId) (:60) and disposeAll() (:67) as the seam
  • On a network change, reset the herd screen's failure backoff (_HostHerd.failStreak
    app/lib/src/screens/herd_screen.dart:97 and nextPollAt :100) so the next tick
    retries immediately
  • Invalidate for all hosts, not just the focused one — the registry is keyed by
    hostId and every cached client is equally stale after an interface change
  • Tests for the invalidation + backoff-reset behaviour driven by a faked signal

Out (do NOT):

  • Do NOT retry aggressively or add a reconnect loop on the network-change event. The OS
    signal means "the interface changed", NOT "the host is reachable". These hosts are
    reached over Tailscale, which needs its own time to re-establish on the new interface;
    hammering it would make things worse. Invalidate and reset, then let the existing
    periodic poll do the retry.
  • Do NOT add a "reconnecting…" UI state or change any user-facing error copy — that is a
    separate UX decision and belongs in a follow-up, not this PR
  • Do NOT change _execTimeout (15s) or the SSHSocket.connect 10s timeout
  • Do NOT add failure backoff to agent_screen.dart — it deliberately has none today
    (it polls every 2s under a _loading guard, :262/:387)
  • Do NOT bound the auth handshake wait — that is issue fix(ssh): bound the SSH auth handshake wait in SshCommandRunner._connect() #118, shipped separately

Implementation notes

  • New dependency: connectivity_plus in app/pubspec.yaml. Wrap its stream in a small
    interface (e.g. an abstract class with a Stream<void> get changes) so tests inject a
    fake rather than depending on platform channels.
  • The registry is the natural owner of invalidation — see the existing evict/disposeAll
    and the comment at host_connections.dart:24 about atomicity (map mutation is sync, only
    the dispose is async). Preserve that property.
  • herd_screen.dart:373 already clears a host's error state including the poll backoff
    and reloads; reuse that logic rather than writing a second reset path.
  • Debounce or coalesce the signal — iOS emits several connectivity events during a single
    Wi-Fi transition, and each one should not trigger a separate teardown.
  • connectivity_plus reports interface changes, not reachability. Treat "no connectivity"
    as a reason to stop polling rather than a reason to reconnect.

Acceptance criteria

  • A network-change signal invalidates every cached SSH connection in
    HostConnectionRegistry
  • A network-change signal clears failStreak/nextPollAt for every host on the herd
    screen, so the next poll tick issues a real request
  • Repeated connectivity events during one transition do not cause repeated teardowns
    (debounced/coalesced)
  • The signal source is injectable — tests drive it with a fake, no platform channels
  • No aggressive reconnect loop is introduced; retries still come from the existing
    periodic poll
  • cd app && fvm flutter analyze passes
  • All tests pass (cd app && fvm flutter test)

References

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions