Skip to content

Commit b772ef3

Browse files
Copilotstephentoub
andcommitted
Revert terminal task cancellation validation to restore idempotent behavior
Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
1 parent 5b3348f commit b772ef3

File tree

5 files changed

+11
-43
lines changed

5 files changed

+11
-43
lines changed

docs/concepts/tasks/tasks.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ Task operations may throw <xref:ModelContextProtocol.McpException> with these er
424424

425425
| Error Code | Scenario |
426426
|------------|----------|
427-
| `InvalidParams` | Invalid or nonexistent task ID, invalid cursor, or attempting to cancel a task already in a terminal status |
427+
| `InvalidParams` | Invalid or nonexistent task ID or invalid cursor |
428428
| `InvalidParams` | Tool with `taskSupport: forbidden` called with task metadata, or tool with `taskSupport: required` called without task metadata |
429429
| `InternalError` | Task execution failure or result unavailable |
430430

src/ModelContextProtocol.Core/Client/McpClientImpl.cs

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -461,21 +461,6 @@ private void RegisterTaskHandlers(RequestHandlers requestHandlers, IMcpTaskStore
461461
throw new McpProtocolException("Missing required parameter 'taskId'", McpErrorCode.InvalidParams);
462462
}
463463

464-
// Get current task status
465-
var currentTask = await taskStore.GetTaskAsync(taskId, SessionId, cancellationToken).ConfigureAwait(false);
466-
if (currentTask is null)
467-
{
468-
throw new McpProtocolException($"Task not found: '{taskId}'", McpErrorCode.InvalidParams);
469-
}
470-
471-
// Validate not already in terminal status
472-
if (currentTask.Status is McpTaskStatus.Completed or McpTaskStatus.Failed or McpTaskStatus.Cancelled)
473-
{
474-
throw new McpProtocolException(
475-
$"Cannot cancel task '{taskId}' in terminal status '{currentTask.Status}'.",
476-
McpErrorCode.InvalidParams);
477-
}
478-
479464
// Signal cancellation if task is still running
480465
_taskCancellationTokenProvider!.Cancel(taskId);
481466

src/ModelContextProtocol.Core/McpErrorCode.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public enum McpErrorCode
6767
/// <item><description><b>Prompts</b>: Unknown prompt name or missing required arguments.</description></item>
6868
/// <item><description><b>Pagination</b>: Invalid or expired cursor values.</description></item>
6969
/// <item><description><b>Logging</b>: Invalid log level.</description></item>
70-
/// <item><description><b>Tasks</b>: Invalid or nonexistent task ID, invalid cursor, or attempting to cancel a task already in a terminal status.</description></item>
70+
/// <item><description><b>Tasks</b>: Invalid or nonexistent task ID or invalid cursor.</description></item>
7171
/// <item><description><b>Elicitation</b>: Server requests an elicitation mode not declared in client capabilities.</description></item>
7272
/// <item><description><b>Sampling</b>: Missing tool result or tool results mixed with other content.</description></item>
7373
/// </list>

src/ModelContextProtocol.Core/Server/McpServerImpl.cs

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -771,21 +771,6 @@ async Task<JsonElement> GetTaskResultAsync(RequestContext<GetTaskPayloadRequestP
771771
throw new McpProtocolException("Missing required parameter 'taskId'", McpErrorCode.InvalidParams);
772772
}
773773

774-
// Get current task status
775-
var currentTask = await taskStore.GetTaskAsync(taskId, SessionId, cancellationToken).ConfigureAwait(false);
776-
if (currentTask is null)
777-
{
778-
throw new McpProtocolException($"Task not found: '{taskId}'", McpErrorCode.InvalidParams);
779-
}
780-
781-
// Validate not already in terminal status
782-
if (currentTask.Status is McpTaskStatus.Completed or McpTaskStatus.Failed or McpTaskStatus.Cancelled)
783-
{
784-
throw new McpProtocolException(
785-
$"Cannot cancel task '{taskId}' in terminal status '{currentTask.Status}'.",
786-
McpErrorCode.InvalidParams);
787-
}
788-
789774
// Signal cancellation if task is still running
790775
_taskCancellationTokenProvider!.Cancel(taskId);
791776

tests/ModelContextProtocol.Tests/Server/TaskCancellationIntegrationTests.cs

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -460,14 +460,13 @@ public async Task CompletedTask_CannotTransitionToOtherStatus()
460460

461461
Assert.Equal(McpTaskStatus.Completed, taskStatus.Status);
462462

463-
// Act & Assert - Try to cancel a completed task should throw InvalidParams
464-
var exception = await Assert.ThrowsAsync<McpProtocolException>(async () =>
465-
await client.CancelTaskAsync(taskId, cancellationToken: TestContext.Current.CancellationToken));
463+
// Act - Try to cancel a completed task (should be idempotent)
464+
var cancelResult = await client.CancelTaskAsync(taskId, cancellationToken: TestContext.Current.CancellationToken);
466465

467-
Assert.Equal(McpErrorCode.InvalidParams, exception.ErrorCode);
468-
Assert.Contains("terminal status", exception.Message, StringComparison.OrdinalIgnoreCase);
466+
// Assert - Status should still be completed (not cancelled)
467+
Assert.Equal(McpTaskStatus.Completed, cancelResult.Status);
469468

470-
// Verify task status unchanged
469+
// Verify via get
471470
var verifyStatus = await client.GetTaskAsync(taskId, cancellationToken: TestContext.Current.CancellationToken);
472471
Assert.Equal(McpTaskStatus.Completed, verifyStatus.Status);
473472
}
@@ -501,11 +500,10 @@ public async Task FailedTask_CannotTransitionToOtherStatus()
501500

502501
Assert.Equal(McpTaskStatus.Failed, taskStatus.Status);
503502

504-
// Act & Assert - Try to cancel a failed task should throw InvalidParams
505-
var exception = await Assert.ThrowsAsync<McpProtocolException>(async () =>
506-
await client.CancelTaskAsync(taskId, cancellationToken: TestContext.Current.CancellationToken));
503+
// Act - Try to cancel a failed task (should be idempotent)
504+
var cancelResult = await client.CancelTaskAsync(taskId, cancellationToken: TestContext.Current.CancellationToken);
507505

508-
Assert.Equal(McpErrorCode.InvalidParams, exception.ErrorCode);
509-
Assert.Contains("terminal status", exception.Message, StringComparison.OrdinalIgnoreCase);
506+
// Assert - Status should still be failed
507+
Assert.Equal(McpTaskStatus.Failed, cancelResult.Status);
510508
}
511509
}

0 commit comments

Comments
 (0)