ListTasksAsync_KeysetPaginationMaintainsConsistencyWithNewTasks failing sporadically in CI

[stephentoub]

[xUnit.net 00:00:25.03]     ModelContextProtocol.Tests.Server.InMemoryMcpTaskStoreTests.ListTasksAsync_KeysetPaginationMaintainsConsistencyWithNewTasks [FAIL]
  Failed ModelContextProtocol.Tests.Server.InMemoryMcpTaskStoreTests.ListTasksAsync_KeysetPaginationMaintainsConsistencyWithNewTasks [11 ms]
  Error Message:
   Assert.NotNull() Failure: Value is null
  Stack Trace:
     at ModelContextProtocol.Tests.Server.InMemoryMcpTaskStoreTests.<ListTasksAsync_KeysetPaginationMaintainsConsistencyWithNewTasks>d__41.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)

[ericstj]

I hit this myself - I think this is suspect:

csharp-sdk/src/ModelContextProtocol.Core/Server/InMemoryMcpTaskStore.cs

Line 344 in 7c85ac6

query = query.Where(e => (e.CreatedAt, e.TaskId).CompareTo(parsedCursorValue) > 0);

What if tasks have identical CreatedAt values, it will then compare the Ids, which are Guid strings and have no numerical relationship. I think DateTimeOffset.UtcNow doesn't have a high-enough resolution to ensure tasks won't get the same CreatedAt value, then there is nothing to ensure a task added later (with same CreatedAt) might sort lower than a cursor boundary already handed out.

csharp-sdk/src/ModelContextProtocol.Core/Server/InMemoryMcpTaskStore.cs

Line 420 in 7c85ac6

private static string GenerateTaskId() => Guid.NewGuid().ToString("N");

The test would fail in this case, if all of the last 5 tasks added after the first cursor boundary was established sorted below that boundary.

@eiriktsarpalis can you review this algorithm and see what you intended for this? Did you mean to use something other than a GUID that might ensure monotonic increasing Task IDs?

[eiriktsarpalis]

This check was meant to be run after the sorting operation that currently happens in line 349. Pretty certain it used to do this, so wondering if copilot at some point reshuffled this part without me noticing.