From 0f64d834b4cb991e616bb831b3dba0b576789644 Mon Sep 17 00:00:00 2001
From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com>
Date: Sat, 25 Jul 2026 11:11:48 -0700
Subject: [PATCH] test(crowdsec): pin the drain task to the loop's lifetime

Every other reporter test reaches Stop() without a Start(), so the drain task
was never exercised and the Task<ValueTask> defect passed the whole suite while
doing nothing. Race the drain against a delay: with an empty queue the loop
parks on WaitToReadAsync forever, so a correctly unwrapped task cannot win that
race, while the Task<ValueTask> completed in ~0ms. Then assert Stop() does not
return until the loop has actually exited, which is the precondition the flush
relies on before reading the SingleReader channel.

Verified by reverting DrainLoop to ValueTask: the test fails. A slow pool can
only make it under-detect, never fail spuriously.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .../Network/Bans/CrowdSecReporterTests.cs     | 38 +++++++++++++++++++
 .../Misc/CrowdSec/CrowdSecReporter.cs         |  6 +++
 2 files changed, 44 insertions(+)

diff --git a/Projects/UOContent.Tests/Tests/Network/Bans/CrowdSecReporterTests.cs b/Projects/UOContent.Tests/Tests/Network/Bans/CrowdSecReporterTests.cs
index 937be3fb5..c6b83706f 100644
--- a/Projects/UOContent.Tests/Tests/Network/Bans/CrowdSecReporterTests.cs
+++ b/Projects/UOContent.Tests/Tests/Network/Bans/CrowdSecReporterTests.cs
@@ -160,6 +160,44 @@ public class CrowdSecReporterTests
         Assert.Equal(0, reporter.SendFailureCount);
     }
 
+    /// <summary>
+    /// Regression for the two shapes that made the drain task lie about the loop's lifetime.
+    /// </summary>
+    /// <remarks>
+    /// <see cref="CrowdSecReporter.Start"/> hands the drain loop to <see cref="Task.Run(Func{Task})"/>,
+    /// and there is no <c>Task.Run(Func&lt;ValueTask&gt;)</c> overload — a ValueTask-returning drain loop
+    /// binds to <c>Task.Run&lt;TResult&gt;(Func&lt;TResult&gt;)</c> and yields a <c>Task&lt;ValueTask&gt;</c>
+    /// that completes at the first suspending await rather than when the loop exits, silently upcast by
+    /// the <see cref="Task"/> field. That turned <see cref="CrowdSecReporter.Stop"/>'s drain-exited
+    /// handshake into a no-op and let the shutdown flush read the <c>SingleReader</c> channel while the
+    /// drain loop was still reading it. Every other test here reaches Stop() without a Start(), so
+    /// nothing covered it.
+    /// <para>
+    /// Racing the drain against a delay is what separates the two shapes: with an empty queue the loop
+    /// parks on <c>WaitToReadAsync</c> forever, so a correctly unwrapped task cannot win that race, while
+    /// the <c>Task&lt;ValueTask&gt;</c> completed in ~0ms. A slow pool can only make this under-detect,
+    /// never fail spuriously.
+    /// </para>
+    /// </remarks>
+    [Fact]
+    public async Task Start_DrainTaskSpansLoopLifetime_NotJustTheFirstAwait()
+    {
+        var reporter = new CrowdSecReporter(new NullAlertClient(), Settings());
+        using var cts = new CancellationTokenSource();
+
+        reporter.Start(cts.Token);
+
+        var drain = reporter.DrainTaskForTesting;
+        Assert.NotNull(drain);
+
+        var first = await Task.WhenAny(drain, Task.Delay(TimeSpan.FromMilliseconds(500)));
+        Assert.False(ReferenceEquals(first, drain), "drain task completed while the loop was still running");
+
+        reporter.Stop();
+
+        Assert.True(drain.IsCompleted, "Stop() returned before the drain loop exited");
+    }
+
     private sealed class NullAlertClient : ICrowdSecAlertClient
     {
         public ValueTask PostAlertsAsync(IReadOnlyList<CrowdSecAlert> alerts, CancellationToken token) =>
diff --git a/Projects/UOContent/Misc/CrowdSec/CrowdSecReporter.cs b/Projects/UOContent/Misc/CrowdSec/CrowdSecReporter.cs
index 84cfbdf31..77334f02f 100644
--- a/Projects/UOContent/Misc/CrowdSec/CrowdSecReporter.cs
+++ b/Projects/UOContent/Misc/CrowdSec/CrowdSecReporter.cs
@@ -69,6 +69,12 @@ public sealed class CrowdSecReporter : IBanReporter
     /// </summary>
     public int SendFailureCount => _sendFailures;
 
+    /// <summary>
+    /// The drain loop's task, so tests can observe its lifetime. It must stay incomplete until the loop
+    /// actually exits — see the note on <see cref="DrainLoop"/> for the shape that silently broke that.
+    /// </summary>
+    internal Task DrainTaskForTesting => _drainTask;
+
     public static void Configure()
     {
         BanChannel.Register(new CrowdSecReporter());