diff --git a/src/Common/Polyfills/System/IdHelpers.cs b/src/Common/Polyfills/System/IdHelpers.cs new file mode 100644 index 000000000..a698ccada --- /dev/null +++ b/src/Common/Polyfills/System/IdHelpers.cs @@ -0,0 +1,40 @@ +using System.Threading; + +namespace System; + +/// +/// Provides helper methods for monotonic ID generation. +/// +internal static class IdHelpers +{ + private static long s_counter; + + /// + /// Creates a strictly monotonically increasing identifier string using 64-bit timestamp ticks + /// and a 64-bit counter, formatted as a 32-character hexadecimal string (GUID-like). + /// + /// The timestamp to embed in the identifier. + /// A new strictly monotonically increasing identifier string. + /// + /// + /// This method creates a 128-bit identifier composed of two 64-bit values: + /// - High 64 bits: from the timestamp + /// - Low 64 bits: A globally monotonically increasing counter + /// + /// + /// The resulting string is strictly monotonically increasing when compared lexicographically, + /// which is required for keyset pagination to work correctly. Unlike Guid.CreateVersion7, + /// which uses random bits for intra-millisecond uniqueness, this implementation guarantees + /// strict ordering for all identifiers regardless of when they were created. + /// + /// + public static string CreateMonotonicId(DateTimeOffset timestamp) + { + long ticks = timestamp.UtcTicks; + long counter = Interlocked.Increment(ref s_counter); + + // Format as 32-character hex string (16 bytes = 128 bits) + // High 64 bits: timestamp ticks, Low 64 bits: counter + return $"{ticks:x16}{counter:x16}"; + } +} diff --git a/src/ModelContextProtocol.Core/ModelContextProtocol.Core.csproj b/src/ModelContextProtocol.Core/ModelContextProtocol.Core.csproj index 63b7bbd45..bc4ae4c81 100644 --- a/src/ModelContextProtocol.Core/ModelContextProtocol.Core.csproj +++ b/src/ModelContextProtocol.Core/ModelContextProtocol.Core.csproj @@ -7,7 +7,6 @@ ModelContextProtocol.Core Core .NET SDK for the Model Context Protocol (MCP) README.md - True $(NoWarn);MCPEXP001 @@ -19,6 +18,7 @@ $(NoWarn);CS0436 + true diff --git a/src/ModelContextProtocol.Core/Server/InMemoryMcpTaskStore.cs b/src/ModelContextProtocol.Core/Server/InMemoryMcpTaskStore.cs index c24b78961..27156e98d 100644 --- a/src/ModelContextProtocol.Core/Server/InMemoryMcpTaskStore.cs +++ b/src/ModelContextProtocol.Core/Server/InMemoryMcpTaskStore.cs @@ -3,7 +3,11 @@ using System.Diagnostics.CodeAnalysis; using System.Text.Json; +#if MCP_TEST_TIME_PROVIDER +namespace ModelContextProtocol.Tests.Internal; +#else namespace ModelContextProtocol; +#endif /// /// Provides an in-memory implementation of for development and testing. @@ -35,6 +39,9 @@ public sealed class InMemoryMcpTaskStore : IMcpTaskStore, IDisposable private readonly int _pageSize; private readonly int? _maxTasks; private readonly int? _maxTasksPerSession; +#if MCP_TEST_TIME_PROVIDER + private readonly TimeProvider _timeProvider; +#endif /// /// Initializes a new instance of the class. @@ -120,6 +127,9 @@ public InMemoryMcpTaskStore( _pageSize = pageSize; _maxTasks = maxTasks; _maxTasksPerSession = maxTasksPerSession; +#if MCP_TEST_TIME_PROVIDER + _timeProvider = TimeProvider.System; +#endif cleanupInterval ??= TimeSpan.FromMinutes(1); if (cleanupInterval.Value != Timeout.InfiniteTimeSpan) @@ -128,6 +138,26 @@ public InMemoryMcpTaskStore( } } +#if MCP_TEST_TIME_PROVIDER + /// + /// Initializes a new instance of the class with a custom time provider. + /// This constructor is only available for testing purposes. + /// + internal InMemoryMcpTaskStore( + TimeSpan? defaultTtl, + TimeSpan? maxTtl, + TimeSpan? pollInterval, + TimeSpan? cleanupInterval, + int pageSize, + int? maxTasks, + int? maxTasksPerSession, + TimeProvider timeProvider) + : this(defaultTtl, maxTtl, pollInterval, cleanupInterval, pageSize, maxTasks, maxTasksPerSession) + { + _timeProvider = timeProvider ?? TimeProvider.System; + } +#endif + /// public Task CreateTaskAsync( McpTaskMetadata taskParams, @@ -155,7 +185,7 @@ public Task CreateTaskAsync( } var taskId = GenerateTaskId(); - var now = DateTimeOffset.UtcNow; + var now = GetUtcNow(); // Determine TTL: use requested, fall back to default, respect max limit var ttl = taskParams.TimeToLive ?? _defaultTtl; @@ -242,7 +272,7 @@ public Task StoreTaskResultAsync( var updatedEntry = new TaskEntry(entry) { Status = status, - LastUpdatedAt = DateTimeOffset.UtcNow, + LastUpdatedAt = GetUtcNow(), StoredResult = result }; @@ -303,7 +333,7 @@ public Task UpdateTaskStatusAsync( { Status = status, StatusMessage = statusMessage, - LastUpdatedAt = DateTimeOffset.UtcNow, + LastUpdatedAt = GetUtcNow(), }; if (_tasks.TryUpdate(taskId, updatedEntry, entry)) @@ -321,32 +351,22 @@ public Task ListTasksAsync( string? sessionId = null, CancellationToken cancellationToken = default) { - // Parse cursor: format is "CreatedAt|TaskId" for keyset pagination - (DateTimeOffset, string)? parsedCursor = null; - if (cursor != null) - { - var parts = cursor.Split('|'); - if (parts.Length == 2 && - DateTimeOffset.TryParse(parts[0], out var parsedDate)) - { - parsedCursor = (parsedDate, parts[1]); - } - } - // Stream enumeration - filter by session, exclude expired, apply keyset pagination var query = _tasks.Values .Where(e => sessionId == null || e.SessionId == sessionId) .Where(e => !IsExpired(e)); - // Apply keyset filter if cursor provided: (CreatedAt, TaskId) > cursor - if (parsedCursor is { } parsedCursorValue) + // Apply keyset filter if cursor provided: TaskId > cursor + // UUID v7 task IDs are monotonically increasing and inherently time-ordered + if (cursor != null) { - query = query.Where(e => (e.CreatedAt, e.TaskId).CompareTo(parsedCursorValue) > 0); + query = query.Where(e => string.CompareOrdinal(e.TaskId, cursor) > 0); } - // Order by (CreatedAt, TaskId) for stable, deterministic pagination + // Order by TaskId for stable, deterministic pagination + // UUID v7 task IDs sort chronologically due to embedded timestamp var page = query - .OrderBy(e => (e.CreatedAt, e.TaskId)) + .OrderBy(e => e.TaskId, StringComparer.Ordinal) .Take(_pageSize + 1) // Take one extra to check if there's a next page .Select(e => e.ToMcpTask()) .ToList(); @@ -356,7 +376,7 @@ public Task ListTasksAsync( if (page.Count > _pageSize) { var lastItemInPage = page[_pageSize - 1]; // Last item we'll actually return - nextCursor = $"{lastItemInPage.CreatedAt:O}|{lastItemInPage.TaskId}"; + nextCursor = lastItemInPage.TaskId; page.RemoveAt(_pageSize); // Remove the extra item } else @@ -397,7 +417,7 @@ public Task CancelTaskAsync(string taskId, string? sessionId = null, Ca var updatedEntry = new TaskEntry(entry) { Status = McpTaskStatus.Cancelled, - LastUpdatedAt = DateTimeOffset.UtcNow, + LastUpdatedAt = GetUtcNow(), }; if (_tasks.TryUpdate(taskId, updatedEntry, entry)) @@ -417,12 +437,23 @@ public void Dispose() _cleanupTimer?.Dispose(); } - private static string GenerateTaskId() => Guid.NewGuid().ToString("N"); + private string GenerateTaskId() => + IdHelpers.CreateMonotonicId(GetUtcNow()); private static bool IsTerminalStatus(McpTaskStatus status) => status is McpTaskStatus.Completed or McpTaskStatus.Failed or McpTaskStatus.Cancelled; +#if MCP_TEST_TIME_PROVIDER + private DateTimeOffset GetUtcNow() => _timeProvider.GetUtcNow(); +#else + private static DateTimeOffset GetUtcNow() => DateTimeOffset.UtcNow; +#endif + +#if MCP_TEST_TIME_PROVIDER + private bool IsExpired(TaskEntry entry) +#else private static bool IsExpired(TaskEntry entry) +#endif { if (entry.TimeToLive == null) { @@ -430,7 +461,7 @@ private static bool IsExpired(TaskEntry entry) } var expirationTime = entry.CreatedAt + entry.TimeToLive.Value; - return DateTimeOffset.UtcNow >= expirationTime; + return GetUtcNow() >= expirationTime; } private void CleanupExpiredTasks(object? state) diff --git a/tests/ModelContextProtocol.Tests/ModelContextProtocol.Tests.csproj b/tests/ModelContextProtocol.Tests/ModelContextProtocol.Tests.csproj index e0fb3d1fa..7dafe3418 100644 --- a/tests/ModelContextProtocol.Tests/ModelContextProtocol.Tests.csproj +++ b/tests/ModelContextProtocol.Tests/ModelContextProtocol.Tests.csproj @@ -5,12 +5,15 @@ net10.0;net9.0;net8.0;net472 enable enable + true false true ModelContextProtocol.Tests $(NoWarn);NU1903;NU1902 + + $(DefineConstants);MCP_TEST_TIME_PROVIDER @@ -27,6 +30,10 @@ + + + + @@ -34,6 +41,10 @@ + + + + runtime; build; native; contentfiles; analyzers; buildtransitive @@ -43,6 +54,7 @@ + diff --git a/tests/ModelContextProtocol.Tests/Server/InMemoryMcpTaskStoreTests.cs b/tests/ModelContextProtocol.Tests/Server/InMemoryMcpTaskStoreTests.cs index fc858ee1e..5b9db455a 100644 --- a/tests/ModelContextProtocol.Tests/Server/InMemoryMcpTaskStoreTests.cs +++ b/tests/ModelContextProtocol.Tests/Server/InMemoryMcpTaskStoreTests.cs @@ -1,7 +1,9 @@ +using Microsoft.Extensions.Time.Testing; using ModelContextProtocol.Protocol; using ModelContextProtocol.Server; using ModelContextProtocol.Tests.Utils; using System.Text.Json; +using TestInMemoryMcpTaskStore = ModelContextProtocol.Tests.Internal.InMemoryMcpTaskStore; namespace ModelContextProtocol.Tests.Server; @@ -1031,4 +1033,141 @@ public async Task CreateTaskAsync_MaxTasksPerSession_ExcludesExpiredTasks() // Assert Assert.NotNull(task2); } + + [Fact] + public async Task ListTasksAsync_KeysetPaginationWorksWithIdenticalTimestamps() + { + // Arrange - Use a fake time provider to create tasks with identical timestamps + var fakeTime = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var store = new TestInMemoryMcpTaskStore( + defaultTtl: null, + maxTtl: null, + pollInterval: null, + cleanupInterval: Timeout.InfiniteTimeSpan, + pageSize: 5, + maxTasks: null, + maxTasksPerSession: null, + timeProvider: fakeTime); + + // Create 10 tasks - all with the EXACT same timestamp + var createdTasks = new List(); + for (int i = 0; i < 10; i++) + { + var task = await store.CreateTaskAsync( + new McpTaskMetadata(), + new RequestId($"req-{i}"), + new JsonRpcRequest { Method = "test" }, + null, + TestContext.Current.CancellationToken); + createdTasks.Add(task); + } + + // Verify all tasks have the same CreatedAt timestamp + var firstTimestamp = createdTasks[0].CreatedAt; + Assert.All(createdTasks, task => Assert.Equal(firstTimestamp, task.CreatedAt)); + + // Act - Get first page + var result1 = await store.ListTasksAsync(cancellationToken: TestContext.Current.CancellationToken); + + // Assert - First page should have 5 tasks + Assert.Equal(5, result1.Tasks.Length); + Assert.NotNull(result1.NextCursor); + + // Get second page using cursor + var result2 = await store.ListTasksAsync(cursor: result1.NextCursor, cancellationToken: TestContext.Current.CancellationToken); + + // Assert - Second page should have 5 tasks + Assert.Equal(5, result2.Tasks.Length); + Assert.Null(result2.NextCursor); // No more pages + + // Verify no overlap between pages + var page1Ids = result1.Tasks.Select(t => t.TaskId).ToHashSet(); + var page2Ids = result2.Tasks.Select(t => t.TaskId).ToHashSet(); + Assert.Empty(page1Ids.Intersect(page2Ids)); + + // Verify we got all 10 tasks exactly once + var allReturnedIds = page1Ids.Union(page2Ids).ToHashSet(); + var allCreatedIds = createdTasks.Select(t => t.TaskId).ToHashSet(); + Assert.Equal(allCreatedIds, allReturnedIds); + } + + [Fact] + public async Task ListTasksAsync_TasksCreatedAfterFirstPageWithSameTimestampAppearInSecondPage() + { + // Arrange - Use a fake time provider so we can control timestamps precisely + var fakeTime = new FakeTimeProvider(DateTimeOffset.UtcNow); + using var store = new TestInMemoryMcpTaskStore( + defaultTtl: null, + maxTtl: null, + pollInterval: null, + cleanupInterval: Timeout.InfiniteTimeSpan, + pageSize: 5, + maxTasks: null, + maxTasksPerSession: null, + timeProvider: fakeTime); + + // Create initial 6 tasks - all with the same timestamp + // (6 so that first page has 5 and cursor points to task 5) + var initialTasks = new List(); + for (int i = 0; i < 6; i++) + { + var task = await store.CreateTaskAsync( + new McpTaskMetadata(), + new RequestId($"req-initial-{i}"), + new JsonRpcRequest { Method = "test" }, + null, + TestContext.Current.CancellationToken); + initialTasks.Add(task); + } + + // Get first page - should have 5 tasks with a cursor + var result1 = await store.ListTasksAsync(cancellationToken: TestContext.Current.CancellationToken); + Assert.Equal(5, result1.Tasks.Length); + Assert.NotNull(result1.NextCursor); + + // Now create 5 more tasks AFTER we got the first page cursor + // These tasks have the SAME timestamp as the cursor (time hasn't moved) + // Due to monotonic UUID v7 with counter, they should sort AFTER the cursor + var laterTasks = new List(); + for (int i = 0; i < 5; i++) + { + var task = await store.CreateTaskAsync( + new McpTaskMetadata(), + new RequestId($"req-later-{i}"), + new JsonRpcRequest { Method = "test" }, + null, + TestContext.Current.CancellationToken); + laterTasks.Add(task); + } + + // Verify all tasks have the same timestamp + var allTasks = initialTasks.Concat(laterTasks).ToList(); + var firstTimestamp = allTasks[0].CreatedAt; + Assert.All(allTasks, task => Assert.Equal(firstTimestamp, task.CreatedAt)); + + // Get ALL remaining pages + var allSubsequentTasks = new List(); + string? cursor = result1.NextCursor; + while (cursor != null) + { + var result = await store.ListTasksAsync(cursor: cursor, cancellationToken: TestContext.Current.CancellationToken); + allSubsequentTasks.AddRange(result.Tasks); + cursor = result.NextCursor; + } + + // Verify no overlap between first page and subsequent + var page1Ids = result1.Tasks.Select(t => t.TaskId).ToHashSet(); + var subsequentIds = allSubsequentTasks.Select(t => t.TaskId).ToHashSet(); + Assert.Empty(page1Ids.Intersect(subsequentIds)); + + // Verify we got all tasks + var allReturnedIds = page1Ids.Union(subsequentIds).ToHashSet(); + var allCreatedIds = allTasks.Select(t => t.TaskId).ToHashSet(); + Assert.Equal(allCreatedIds, allReturnedIds); + + // Most importantly: verify ALL the later tasks (created after first page) are surfaced + // in the subsequent pages + var laterTaskIds = laterTasks.Select(t => t.TaskId).ToHashSet(); + Assert.Superset(laterTaskIds, subsequentIds); + } }