fix(site-call-audit): composite keyset cursor eliminates the single-timestamp reconciliation pin; pinned state now a published event

Reconciliation cursor becomes composite (UpdatedAtUtc, TrackedOperationId); IPullSiteCallsClient/GrpcPullSiteCallsClient forward the Task-15 after_id keyset (additive param, null preserves the legacy inclusive >= contract). A burst sharing one exact UpdatedAtUtc now drains via the id tiebreak instead of pinning forever. A legacy site that ignores after_id is latched + published as SiteCallReconciliationPinnedChanged on the EventStream (transition-only), replacing the prior silent log line.
2026-07-09 08:44:07 -04:00
parent 1f4c0b67ca
commit 20098c6108
9 changed files with 324 additions and 66 deletions
@@ -42,7 +42,7 @@ public class SiteCallAuditPurgeTests : TestKit
    private sealed class NoOpPullClient : IPullSiteCallsClient
    {
        public Task<PullSiteCallsResponse> PullAsync(
-            string siteId, DateTime sinceUtc, int batchSize, CancellationToken ct) =>
+            string siteId, DateTime sinceUtc, string? afterId, int batchSize, CancellationToken ct) =>
            Task.FromResult(new PullSiteCallsResponse(Array.Empty<SiteCall>(), MoreAvailable: false));
    }

@@ -74,7 +74,7 @@ public class SiteCallAuditReconciliationTests : TestKit
    /// </summary>
    private sealed class ScriptedPullClient : IPullSiteCallsClient
    {
-        public List<(string SiteId, DateTime SinceUtc, int BatchSize)> Calls { get; } = new();
+        public List<(string SiteId, DateTime SinceUtc, string? AfterId, int BatchSize)> Calls { get; } = new();
        private readonly Dictionary<string, Queue<PullSiteCallsResponse>> _scripted = new();
        private readonly Dictionary<string, Exception> _throwOnSite = new();

@@ -91,9 +91,9 @@ public class SiteCallAuditReconciliationTests : TestKit
        }

        public Task<PullSiteCallsResponse> PullAsync(
-            string siteId, DateTime sinceUtc, int batchSize, CancellationToken ct)
+            string siteId, DateTime sinceUtc, string? afterId, int batchSize, CancellationToken ct)
        {
-            Calls.Add((siteId, sinceUtc, batchSize));
+            Calls.Add((siteId, sinceUtc, afterId, batchSize));
            if (_throwOnSite.TryGetValue(siteId, out var ex))
            {
                throw ex;
@@ -107,6 +107,67 @@ public class SiteCallAuditReconciliationTests : TestKit
        }
    }

+    /// <summary>
+    /// Keyset-honouring in-memory pull client (Task 16). Holds one static set of
+    /// rows and answers each pull as a real site would: returns the next
+    /// <c>batchSize</c> rows strictly greater than the composite
+    /// <c>(sinceUtc, afterId)</c> cursor under the deterministic
+    /// <c>(UpdatedAtUtc, TrackedOperationId ordinal)</c> ordering, with
+    /// <c>MoreAvailable=true</c> when unseen rows remain. This is the modern site
+    /// that DOES honour <c>after_id</c>, so a burst sharing one exact
+    /// <c>UpdatedAtUtc</c> drains fully via the id tiebreak — no single-timestamp
+    /// pin.
+    /// </summary>
+    private sealed class KeysetHonoringPullClient : IPullSiteCallsClient
+    {
+        private readonly IReadOnlyList<SiteCall> _rows;
+        public int CallCount { get; private set; }
+
+        public KeysetHonoringPullClient(IEnumerable<SiteCall> rows) =>
+            _rows = rows
+                .OrderBy(r => r.UpdatedAtUtc)
+                .ThenBy(r => r.TrackedOperationId.ToString(), StringComparer.Ordinal)
+                .ToList();
+
+        public Task<PullSiteCallsResponse> PullAsync(
+            string siteId, DateTime sinceUtc, string? afterId, int batchSize, CancellationToken ct)
+        {
+            CallCount++;
+            var greater = _rows
+                .Where(r =>
+                    r.UpdatedAtUtc > sinceUtc ||
+                    (r.UpdatedAtUtc == sinceUtc &&
+                     string.CompareOrdinal(r.TrackedOperationId.ToString(), afterId) > 0))
+                .ToList();
+            var page = greater.Take(batchSize).ToList();
+            var more = greater.Count > page.Count;
+            return Task.FromResult(
+                new PullSiteCallsResponse(page, MoreAvailable: more));
+        }
+    }
+
+    /// <summary>
+    /// Legacy pull client (Task 16) that IGNORES <c>afterId</c> and always
+    /// returns the same saturated page (<c>MoreAvailable=true</c>) — models a
+    /// pre-keyset site the central actor cannot drain past. The composite cursor
+    /// cannot advance, so the actor must latch and publish
+    /// <see cref="SiteCallReconciliationPinnedChanged"/> instead of spinning.
+    /// </summary>
+    private sealed class LegacyIgnoresAfterIdPullClient : IPullSiteCallsClient
+    {
+        private readonly IReadOnlyList<SiteCall> _rows;
+        public int CallCount { get; private set; }
+
+        public LegacyIgnoresAfterIdPullClient(IReadOnlyList<SiteCall> rows) => _rows = rows;
+
+        public Task<PullSiteCallsResponse> PullAsync(
+            string siteId, DateTime sinceUtc, string? afterId, int batchSize, CancellationToken ct)
+        {
+            CallCount++;
+            return Task.FromResult(new PullSiteCallsResponse(_rows, MoreAvailable: true));
+        }
+    }
+
    /// <summary>
    /// Pull client that ALWAYS returns the same saturated response
    /// (<c>MoreAvailable=true</c>) regardless of the <c>since</c> cursor —
@@ -124,7 +185,7 @@ public class SiteCallAuditReconciliationTests : TestKit
        public SaturatedPinPullClient(IReadOnlyList<SiteCall> rows) => _rows = rows;

        public Task<PullSiteCallsResponse> PullAsync(
-            string siteId, DateTime sinceUtc, int batchSize, CancellationToken ct)
+            string siteId, DateTime sinceUtc, string? afterId, int batchSize, CancellationToken ct)
        {
            CallCount++;
            return Task.FromResult(new PullSiteCallsResponse(_rows, MoreAvailable: true));
@@ -436,4 +497,89 @@ public class SiteCallAuditReconciliationTests : TestKit
            $"a single-timestamp saturation pin must break the within-tick drain, not spin to the "
            + $"page ceiling; got {client.CallCount} pulls (an unbounded within-tick loop would be 50+)");
    }
+
+    // ---------------------------------------------------------------------
+    // 7. Task 16: a keyset-honouring site drains a single-timestamp burst
+    //    LARGER than the batch size in ONE tick — the composite (UpdatedAtUtc,
+    //    TrackedOperationId) cursor advances by id even when every row shares
+    //    one exact timestamp, so the tail no longer pins forever.
+    // ---------------------------------------------------------------------
+
+    [Fact]
+    public void ReconciliationTick_KeysetCursor_DrainsSingleTimestampBurstBeyondBatchSize()
+    {
+        var siteId = "siteA";
+        // 2 × batchSize rows all sharing ONE exact UpdatedAtUtc: the pre-keyset
+        // inclusive-timestamp cursor could never advance past this window.
+        var ts = new DateTime(2026, 5, 20, 10, 0, 0, DateTimeKind.Utc);
+        var rows = Enumerable.Range(0, 4)
+            .Select(_ => NewRow(TrackedOperationId.New(), siteId, updatedAtUtc: ts))
+            .ToList();
+
+        var sites = new StaticEnumerator(new SiteEntry(siteId, "http://siteA:8083"));
+        var client = new KeysetHonoringPullClient(rows);
+        var repo = new RecordingRepo();
+
+        // batchSize = 2 → the 4-row burst needs the id tiebreak to page through.
+        // Slow tick so the full drain must be the within-tick continuation loop,
+        // not multiple ticks.
+        var options = new SiteCallAuditOptions
+        {
+            ReconciliationIntervalOverride = TimeSpan.FromSeconds(2),
+            ReconciliationBatchSize = 2,
+        };
+
+        CreateActor(sites, client, repo, options);
+
+        AwaitAssert(
+            () =>
+            {
+                foreach (var r in rows)
+                {
+                    Assert.True(repo.Upserted.ContainsKey(r.TrackedOperationId),
+                        "every row in the single-timestamp burst must reconcile via the "
+                        + "composite keyset cursor within one tick");
+                }
+            },
+            duration: TimeSpan.FromSeconds(3),
+            interval: TimeSpan.FromMilliseconds(50));
+
+        // Two pages (2 + 2) drained the burst; well under the 50-page ceiling.
+        Assert.True(client.CallCount is >= 2 and < 10,
+            $"expected the burst to drain in a couple of keyset pages, got {client.CallCount}");
+    }
+
+    // ---------------------------------------------------------------------
+    // 8. Task 16: a LEGACY site that ignores after_id can never be drained, so
+    //    the actor latches and publishes SiteCallReconciliationPinnedChanged
+    //    instead of silently logging (the dead-end is now health-observable).
+    // ---------------------------------------------------------------------
+
+    [Fact]
+    public void ReconciliationTick_LegacySiteIgnoresAfterId_PublishesPinnedChanged()
+    {
+        var siteId = "siteLegacy";
+        var ts = new DateTime(2026, 5, 20, 10, 0, 0, DateTimeKind.Utc);
+        var r1 = NewRow(TrackedOperationId.New(), siteId, updatedAtUtc: ts);
+        var r2 = NewRow(TrackedOperationId.New(), siteId, updatedAtUtc: ts);
+
+        var sites = new StaticEnumerator(new SiteEntry(siteId, "http://siteLegacy:8083"));
+        var client = new LegacyIgnoresAfterIdPullClient(new[] { r1, r2 });
+        var repo = new RecordingRepo();
+
+        // Subscribe BEFORE the actor starts ticking so the transition isn't missed.
+        Sys.EventStream.Subscribe(TestActor, typeof(SiteCallReconciliationPinnedChanged));
+
+        var options = new SiteCallAuditOptions
+        {
+            ReconciliationIntervalOverride = TimeSpan.FromMilliseconds(100),
+            ReconciliationBatchSize = 2,
+        };
+
+        CreateActor(sites, client, repo, options);
+
+        var evt = ExpectMsg<SiteCallReconciliationPinnedChanged>(TimeSpan.FromSeconds(5));
+        Assert.Equal(siteId, evt.SiteId);
+        Assert.True(evt.Pinned, "a legacy site that ignores after_id must publish Pinned=true");
+    }
 }