refactor(connections): apply PR review feedback

- WatchLivenessAsync now catches the cancellation OperationCanceledException and
  returns ReconnectReason.Stopped, so on shutdown the WhenAny winner is a clean
  reason instead of a faulted task (its doc already advertised Stopped).
- Reword the fake's monuments-timeout helpers/test: the rig/monuments fetch runs
  in the background marker poll now, not on the connect path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: HandyS11

src/RustPlusBot.Features.Connections/Supervisor/ConnectionSupervisor.cs

@@ -658,15 +658,23 @@ private async Task<ReconnectReason> WatchLivenessAsync(
         IRustServerConnection connection,
         CancellationToken ct)
     {
-        while (!ct.IsCancellationRequested)
+        try
         {
-            await Task.Delay(_options.LivenessPollInterval, ct).ConfigureAwait(false);
-            if (!connection.IsConnected)
+            while (!ct.IsCancellationRequested)
             {
-                LogSocketDropped(logger, key.Server);
-                return ReconnectReason.Unreachable;
+                await Task.Delay(_options.LivenessPollInterval, ct).ConfigureAwait(false);
+                if (!connection.IsConnected)
+                {
+                    LogSocketDropped(logger, key.Server);
+                    return ReconnectReason.Unreachable;
+                }
             }
         }
+        catch (OperationCanceledException)
+        {
+            // The connected window is ending (stop/reconnect): Task.Delay throws on cancellation.
+            // Return Stopped so the WhenAny winner is a clean reason rather than a faulted task.
+        }
 
         return ReconnectReason.Stopped;
     }

tests/RustPlusBot.Features.Connections.Tests/ConnectionSupervisorTests.cs

@@ -233,17 +233,17 @@ public async Task Heartbeat_Unreachable_ReconnectsAndRecovers()
     }
 
     /// <summary>
-    /// A per-request timeout during connect-time setup (here the oil-rig monuments fetch) surfaces as an
-    /// <see cref="OperationCanceledException"/> even though the connection token is not cancelled. It must
-    /// degrade rig detection for this window, NOT terminate the whole connection loop — otherwise a
+    /// A per-request timeout in a connected window (here the marker poll's oil-rig monuments fetch) surfaces
+    /// as an <see cref="OperationCanceledException"/> even though the connection token is not cancelled. It
+    /// must degrade rig detection for this window, NOT terminate the whole connection loop — otherwise a
     /// transient slow map endpoint permanently kills the connection with no reconnect (the real-world bug).
     /// </summary>
     [Fact]
     public async Task Connect_MonumentsTimeout_DoesNotKillLoop_AndStillReconnects()
     {
         var source = new FakeRustSocketSource();
         source.EnqueueConnect(SocketConnectOutcome.Connected);
-        source.TimeoutOnMonumentsOnce(); // connect-time rig fetch times out on the FIRST connection only
+        source.TimeoutOnMonumentsOnce(); // the marker poll's rig fetch times out on the FIRST connection only
         source.EnqueueHeartbeat(HeartbeatResult.Ok(2)); // first heartbeat -> Connected
         source.EnqueueHeartbeat(HeartbeatResult.Unreachable); // next heartbeat -> drop
         source.EnqueueConnect(SocketConnectOutcome.Connected); // reconnect

tests/RustPlusBot.Features.Connections.Tests/Fakes/FakeRustSocketSource.cs

@@ -61,7 +61,7 @@ public IRustServerConnection Create(string ip, int port, ulong steamId, string p
         connection.MonumentsResult = _pendingMonuments;
         _pendingMonuments = [];
 
-        // Transfer any pre-staged monuments timeout so the connect-time rig fetch throws for the NEXT
+        // Transfer any pre-staged monuments timeout so the marker poll's rig fetch throws for the NEXT
         // connection only (mimicking a real per-request timeout, which surfaces as OperationCanceledException).
         connection.MonumentsTimeout = _pendingMonumentsTimeout;
         _pendingMonumentsTimeout = false;
@@ -126,10 +126,10 @@ public void EnqueueMarkers(IReadOnlyList<MapMarkerSnapshot> markers) =>
     public void SetMonuments(IReadOnlyList<MonumentSnapshot> monuments) => _pendingMonuments = monuments;
 
     /// <summary>
-    /// Makes the NEXT connection's connect-time <see cref="FakeConnection.GetMonumentsAsync"/> throw
+    /// Makes the NEXT connection's <see cref="FakeConnection.GetMonumentsAsync"/> throw
     /// <see cref="OperationCanceledException"/>, simulating a per-request timeout (the outer connection
-    /// token is NOT cancelled). Applies to the next connection only. Call before
-    /// <see cref="EnsureConnectionAsync"/>.
+    /// token is NOT cancelled). The supervisor issues this fetch from its background marker poll, not the
+    /// connect path. Applies to the next connection only. Call before <see cref="EnsureConnectionAsync"/>.
     /// </summary>
     public void TimeoutOnMonumentsOnce() => _pendingMonumentsTimeout = true;