Skip to content

Enable complete Tasks extension conformance coverage#1741

Open
PranavSenthilnathan wants to merge 1 commit into
modelcontextprotocol:mainfrom
PranavSenthilnathan:pranavsenthilnathan-complete-task-conformance
Open

Enable complete Tasks extension conformance coverage#1741
PranavSenthilnathan wants to merge 1 commit into
modelcontextprotocol:mainfrom
PranavSenthilnathan:pranavsenthilnathan-complete-task-conformance

Conversation

@PranavSenthilnathan

Copy link
Copy Markdown
Contributor

Summary

  • add Tasks-owned per-call execution modes for synchronous, optional, and required task execution
  • allow WithTasks callers to select execution behavior from the tool request without introducing Tasks dependencies in Core or ASP.NET Core
  • enable all ten SEP-2663 Tasks extension conformance scenarios
  • cover mode selection, capability enforcement, legacy protocol behavior, and invalid configuration

Validation

  • dotnet build (0 warnings, 0 errors)
  • all ten Tasks conformance scenarios on .NET 8, .NET 9, and .NET 10 using @modelcontextprotocol/conformance@0.2.0-alpha.9
  • full Core tests on .NET 8, .NET 9, .NET 10, and .NET Framework 4.7.2
  • full ASP.NET Core tests on .NET 8, .NET 9, and .NET 10
  • analyzer and experimental API regression tests

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 7c99928c-a943-49f4-9a63-8b04a0e4d18b
@PranavSenthilnathan
PranavSenthilnathan marked this pull request as ready for review July 25, 2026 02:55
public static string SyncTool() => "sync";

[McpServerTool(Name = "required-tool")]
public static string RequiredTool() => "required";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do I understand correctly?

  1. This return value is never consumed and it's immaterial
  2. "required-tool" gets recognized by the store, treating the tool as required from the client
  3. When the client doesn't have the tool, the 2 tests above assert we throw expectedly
  4. When the client does have the tool, we simply assert that it gets registered as a task
  5. Not much value in asserting the "required" result since that's the same coverage as the "sync" assert?

If that's all correctly, I suggest we go ahead and assert the "required" result anyway, to close this loop for illustration/documentation. Can't hurt to assert the result too, just in case something in the required tool mechanism ever interfered with the result.

@jeffhandley jeffhandley left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one minor question/suggestion.

@tarekgh tarekgh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modulo @jeffhandley comment, LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants