Keep a heartbeat loop alive when its error reporter throws

backend/src/CodeSpace.Core/Services/Agents/HeartbeatLoop.cs

@@ -12,8 +12,9 @@ public static class HeartbeatLoop
     /// <summary>
     /// Wait <paramref name="interval"/>, then invoke <paramref name="ping"/>; repeat until
     /// <paramref name="cancellationToken"/> fires. A ping that throws (a transient DB blip) is reported to
-    /// <paramref name="onPingError"/> and the loop continues — a missed heartbeat must never kill liveness.
-    /// Returns cleanly when cancelled; never surfaces <see cref="OperationCanceledException"/> to the caller.
+    /// <paramref name="onPingError"/> and the loop continues — a missed heartbeat must never kill liveness, and neither
+    /// may a reporter that throws in turn. So the loop completes only by its own cancellation, and then cleanly: never
+    /// faulted, never surfacing <see cref="OperationCanceledException"/> to the caller.
     /// The first ping is deferred by one interval because the claim already stamped an initial heartbeat.
     ///
     /// <para><paramref name="timeProvider"/> exists so the cadence can be driven deterministically in a test instead
@@ -45,7 +46,7 @@ public static async Task RunAsync(Func<CancellationToken, Task> ping, TimeSpan i
                 }
                 catch (Exception ex)
                 {
-                    onPingError(ex);
+                    ReportQuietly(onPingError, ex);
                 }
             }
         }
@@ -54,4 +55,22 @@ public static async Task RunAsync(Func<CancellationToken, Task> ping, TimeSpan i
             // Expected: the harness finished or the worker is stopping. Not an error.
         }
     }
+
+    /// <summary>
+    /// Hands a failed ping to the caller's reporter and lets nothing the reporter throws escape. Every caller awaits the
+    /// loop in the <c>finally</c> around the work it protects, so a reporter's fault that faulted the loop would replace
+    /// that work's result with the error of a log line, and a reporter's <see cref="OperationCanceledException"/> caught
+    /// by the loop's own cancellation exit would end it early, stopping liveness while the work ran on.
+    /// </summary>
+    private static void ReportQuietly(Action<Exception> onPingError, Exception exception)
+    {
+        try
+        {
+            onPingError(exception);
+        }
+        catch (Exception)
+        {
+            // The reporter was the one place to say the ping failed, and it failed too; there is nowhere left to say it.
+        }
+    }
 }

backend/tests/CodeSpace.UnitTests/Workflows/HeartbeatLoopTests.cs

@@ -108,6 +108,50 @@ public async Task A_failing_ping_is_reported_but_does_not_kill_the_loop()
         await loop;   // a loop whose every ping threw still returns cleanly on cancel, never surfacing the failure
     }
 
+    /// <summary>
+    /// The reporter is the caller's code, and every caller awaits this loop in the <c>finally</c> around the work it
+    /// protects, so a reporter that throws may end the loop no more than a failing ping may. It did: the report ran outside
+    /// the ping's guard, so a reporter's fault faulted the loop — and the awaiting <c>finally</c> surfaced it IN PLACE of
+    /// the result the loop was keeping alive — while a reporter's <see cref="OperationCanceledException"/> ended the loop
+    /// early and without a word, stopping liveness while the work ran on. Both are a loop that completed by something other
+    /// than its own cancellation, and both red here as a loop that never arms its next beat.
+    /// </summary>
+    [Theory]
+    [InlineData(typeof(InvalidOperationException))]
+    [InlineData(typeof(OperationCanceledException))]
+    public async Task A_reporter_that_throws_neither_faults_nor_ends_the_loop(Type faultType)
+    {
+        var time = new HeartbeatClock();
+        var interval = TimeSpan.FromSeconds(30);
+        var reported = new SemaphoreSlim(0);
+        var pings = 0;
+        using var cts = new CancellationTokenSource();
+
+        // Released BEFORE the reporter throws, since nothing after the throw runs — and after the ping has counted, so
+        // each signal reads a settled count.
+        var loop = HeartbeatLoop.RunAsync(
+            _ => { Interlocked.Increment(ref pings); throw new InvalidOperationException("transient db blip"); },
+            interval,
+            _ => { reported.Release(); throw (Exception)Activator.CreateInstance(faultType, "the reporter itself failed")!; },
+            cts.Token,
+            time);
+
+        for (var i = 1; i <= 3; i++)
+        {
+            await AdvanceOneIntervalAsync(time, reported, interval, i);
+
+            Volatile.Read(ref pings).ShouldBe(i, "a reporter that threw must not stop, skip, or double the cadence");
+        }
+
+        cts.Cancel();
+
+        // Recorded rather than asserted with Should.NotThrowAsync, which passes a CANCELED task without a word. Bounded, so
+        // a loop that ignores the cancel fails instead of hanging.
+        var escaped = await Record.ExceptionAsync(() => loop.WaitAsync(TimeSpan.FromSeconds(10)));
+
+        escaped.ShouldBeNull("the loop completes only by its own cancellation, and quietly — whatever escapes it is what the awaiting finally surfaces in place of the result it protects");
+    }
+
     [Fact]
     public async Task Returns_without_pinging_when_already_cancelled()
     {