Skip to content

Commit b699d69

Browse files
Fix keyset pagination by sorting before applying cursor filter
Co-authored-by: eiriktsarpalis <2813363+eiriktsarpalis@users.noreply.github.com>
1 parent f32e1e1 commit b699d69

1 file changed

Lines changed: 12 additions & 22 deletions

File tree

src/ModelContextProtocol.Core/Server/InMemoryMcpTaskStore.cs

Lines changed: 12 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,6 @@ namespace ModelContextProtocol;
2727
[Experimental(Experimentals.Tasks_DiagnosticId, UrlFormat = Experimentals.Tasks_Url)]
2828
public sealed class InMemoryMcpTaskStore : IMcpTaskStore, IDisposable
2929
{
30-
// Counter used for generating monotonically increasing task IDs, ensuring chronological ordering
31-
// even when tasks are created within the same millisecond.
32-
private static long s_taskIdCounter;
33-
3430
private readonly ConcurrentDictionary<string, TaskEntry> _tasks = new();
3531
private readonly TimeSpan? _defaultTtl;
3632
private readonly TimeSpan? _maxTtl;
@@ -183,7 +179,7 @@ public Task<McpTask> CreateTaskAsync(
183179

184180
if (!_tasks.TryAdd(taskId, entry))
185181
{
186-
// Should never happen with counter-based IDs since each counter value is unique
182+
// This should be extremely rare with GUID-based IDs
187183
throw new InvalidOperationException($"Task ID collision: {taskId}");
188184
}
189185

@@ -337,20 +333,22 @@ public Task<ListTasksResult> ListTasksAsync(
337333
}
338334
}
339335

340-
// Stream enumeration - filter by session, exclude expired, apply keyset pagination
336+
// Stream enumeration - filter by session, exclude expired
341337
var query = _tasks.Values
342338
.Where(e => sessionId == null || e.SessionId == sessionId)
343339
.Where(e => !IsExpired(e));
344340

341+
// Order by (CreatedAt, TaskId) for stable, deterministic pagination
342+
// Must sort BEFORE applying keyset filter to ensure consistent comparison
343+
var orderedQuery = query.OrderBy(e => (e.CreatedAt, e.TaskId));
344+
345345
// Apply keyset filter if cursor provided: (CreatedAt, TaskId) > cursor
346-
if (parsedCursor is { } parsedCursorValue)
347-
{
348-
query = query.Where(e => (e.CreatedAt, e.TaskId).CompareTo(parsedCursorValue) > 0);
349-
}
346+
// This runs on sorted data, so we skip items until we pass the cursor position
347+
IEnumerable<TaskEntry> filteredQuery = parsedCursor is { } parsedCursorValue
348+
? orderedQuery.SkipWhile(e => (e.CreatedAt, e.TaskId).CompareTo(parsedCursorValue) <= 0)
349+
: orderedQuery;
350350

351-
// Order by (CreatedAt, TaskId) for stable, deterministic pagination
352-
var page = query
353-
.OrderBy(e => (e.CreatedAt, e.TaskId))
351+
var page = filteredQuery
354352
.Take(_pageSize + 1) // Take one extra to check if there's a next page
355353
.Select(e => e.ToMcpTask())
356354
.ToList();
@@ -421,15 +419,7 @@ public void Dispose()
421419
_cleanupTimer?.Dispose();
422420
}
423421

424-
private string GenerateTaskId()
425-
{
426-
// Use Interlocked.Increment to generate a monotonically increasing counter.
427-
// This ensures task IDs maintain chronological ordering for keyset pagination,
428-
// even when multiple tasks are created within the same millisecond.
429-
// Format: {counter:D20}-{uniqueSuffix} where D20 ensures lexicographic sorting.
430-
long counter = Interlocked.Increment(ref s_taskIdCounter);
431-
return $"{counter:D20}-{Guid.NewGuid():N}";
432-
}
422+
private static string GenerateTaskId() => Guid.NewGuid().ToString("N");
433423

434424
private static bool IsTerminalStatus(McpTaskStatus status) =>
435425
status is McpTaskStatus.Completed or McpTaskStatus.Failed or McpTaskStatus.Cancelled;

0 commit comments

Comments
 (0)