Skip to content

The Great Cancellation: Part 1#1056

Merged
anuchandy merged 2 commits intomainfrom
users/vukelich/cancellation
Nov 5, 2025
Merged

The Great Cancellation: Part 1#1056
anuchandy merged 2 commits intomainfrom
users/vukelich/cancellation

Conversation

@vukelich
Copy link
Copy Markdown
Member

@vukelich vukelich commented Nov 4, 2025

What does this PR do?

[Provide a clear, concise description of the changes]

Add a CancellationToken parameter to IBaseCommand.ExecuteAsync.

  • Plumb CancellationToken through most of the product code with CancellationToken.None used temporarily where needed.
  • Update unit tests to use Arg.Any() when mocking methods that now take a CancellationToken parameter.
  • Add documentation guidance for unit tests regarding CancellationToken usage.

More product code and unit tests will be updated in subsequent commits.

[Any additional context, screenshots, or information that helps reviewers]

GitHub issue number?

[Link to the GitHub issue this PR addresses]

Pre-merge Checklist

  • Required for All PRs
    • Read contribution guidelines
    • PR title clearly describes the change
    • Commit history is clean with descriptive messages (cleanup guide)
    • Added comprehensive tests for new/modified functionality
    • Updated servers/Azure.Mcp.Server/CHANGELOG.md and/or servers/Fabric.Mcp.Server/CHANGELOG.md for product changes (features, bug fixes, UI/UX, updated dependencies)
  • For MCP tool changes:
    • One tool per PR: This PR adds or modifies only one MCP tool for faster review cycles
    • Updated servers/Azure.Mcp.Server/README.md and/or servers/Fabric.Mcp.Server/README.md documentation
    • Validate README.md changes using script at eng/scripts/Process-PackageReadMe.ps1. See Package README
    • Updated command list in /servers/Azure.Mcp.Server/docs/azmcp-commands.md and/or /docs/fabric-commands.md
    • Run .\eng\scripts\Update-AzCommandsMetadata.ps1 to update tool metadata in azmcp-commands.md (required for CI)
    • For new or modified tool descriptions, ran ToolDescriptionEvaluator and obtained a score of 0.4 or more and a top 3 ranking for all related test prompts
    • For new tools associated with Azure services or publicly available tools/APIs/products, add URL to documentation in the PR description
  • Extra steps for Azure MCP Server tool changes:
    • Updated test prompts in /servers/Azure.Mcp.Server/docs/e2eTestPrompts.md
    • 👉 For Community (non-Microsoft team member) PRs:
      • Security review: Reviewed code for security vulnerabilities, malicious code, or suspicious activities before running tests (crypto mining, spam, data exfiltration, etc.)
      • Manual tests run: added comment /azp run mcp - pullrequest - live to run Live Test Pipeline

@joshfree joshfree moved this from Untriaged to In Progress in Azure MCP Server Nov 4, 2025
@joshfree joshfree added this to the 2025-11 milestone Nov 4, 2025
Copy link
Copy Markdown
Member

@joshfree joshfree left a comment

Choose a reason for hiding this comment

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

Add a servers/Azure.Mcp.Server/CHANGELOG.md entry as well in this or a follow-up PR

Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
Copy link
Copy Markdown
Contributor

@kvenkatrajan kvenkatrajan left a comment

Choose a reason for hiding this comment

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

Tests are passing. Approving - please ensure the Analyze errors (link warning etc.) fixed and changelog added

Add a CancellationToken parameter to IBaseCommand.ExecuteAsync.

- Plumb CancellationToken through most of the product code with CancellationToken.None used temporarily where needed.
- Update unit tests to use Arg.Any<CancellationToken>() when mocking methods that now take a CancellationToken parameter.
- Add documentation guidance for unit tests regarding CancellationToken usage.

More product code and unit tests will be updated in subsequent commits.
@vukelich vukelich force-pushed the users/vukelich/cancellation branch from 88f5458 to dbd5d32 Compare November 5, 2025 00:53
Comment thread servers/Azure.Mcp.Server/CHANGELOG.md Outdated
Copy link
Copy Markdown
Member

@srnagar srnagar left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for updating the contributing guides and new-command.md.

Copy link
Copy Markdown
Member

@conniey conniey left a comment

Choose a reason for hiding this comment

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

Nice! From what I saw in the files, a CancellationToken parameter was added to each method. That'll be useful. : )

@joshfree
Copy link
Copy Markdown
Member

joshfree commented Nov 5, 2025

CI failed on dotnet format checks

image

@vukelich vukelich enabled auto-merge (squash) November 5, 2025 01:29
@kvenkatrajan kvenkatrajan disabled auto-merge November 5, 2025 03:03
@anuchandy anuchandy merged commit e6463d9 into main Nov 5, 2025
55 checks passed
@anuchandy anuchandy deleted the users/vukelich/cancellation branch November 5, 2025 03:04
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Azure MCP Server Nov 5, 2025
colbytimm pushed a commit to colbytimm/microsoft-mcp that referenced this pull request Dec 8, 2025
* The Great Cancellation: Part 1

Add a CancellationToken parameter to IBaseCommand.ExecuteAsync.

- Plumb CancellationToken through most of the product code with CancellationToken.None used temporarily where needed.
- Update unit tests to use Arg.Any<CancellationToken>() when mocking methods that now take a CancellationToken parameter.
- Add documentation guidance for unit tests regarding CancellationToken usage.

More product code and unit tests will be updated in subsequent commits.

* Typo and adding newline to test files with style errors.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

8 participants