Skip to content

Always set IsServerCommandInvoked to false when first entering CallToolHandler#1718

Merged
alzimmermsft merged 5 commits intomicrosoft:mainfrom
alzimmermsft:InvestigateTelemetryIssues
Feb 13, 2026
Merged

Always set IsServerCommandInvoked to false when first entering CallToolHandler#1718
alzimmermsft merged 5 commits intomicrosoft:mainfrom
alzimmermsft:InvestigateTelemetryIssues

Conversation

@alzimmermsft
Copy link
Copy Markdown
Contributor

@alzimmermsft alzimmermsft commented Feb 12, 2026

What does this PR do?

Fixes cases where IsServerCommandInvoked wasn't being set in some cases. At a high-level the changes are as follows (and all future IToolLoader / BaseToolLoader implementations should follow this):

  • When CallToolHandler is invoked IsServerCommandInvoked should get set to false right away.
  • When IBaseCommand.ExecuteAsync or McpClient.CallToolAsync are about to be called IsServerCommandInvoked should get set to true.
  • If the IToolHandler / BaseToolLoader has learning paths, when those are invoked IsServerCommandInvoked should get set to false no matter what. This handles potential cases where an IsServerCommandInvoked=true cases calls into learning if there was an error.

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 tools with new names, including new tools or renamed tools, update consolidated-tools.json
    • 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

@alzimmermsft alzimmermsft self-assigned this Feb 12, 2026
@alzimmermsft alzimmermsft requested review from a team as code owners February 12, 2026 21:34
@alzimmermsft alzimmermsft added the tools-Core AZMCP Core functionality that all other tools build on top of label Feb 12, 2026
Copilot AI review requested due to automatic review settings February 12, 2026 21:34
Copilot AI reviewed Feb 12, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR standardizes how tool loaders set the TagName.IsServerCommandInvoked activity tag so telemetry reliably distinguishes “learning/listing/routing” from actual server command/tool execution across the MCP server tool-loading pipeline.

Changes:

  • Set IsServerCommandInvoked=false immediately upon entering each CallToolHandler, and set it to true right before executing a command (IBaseCommand.ExecuteAsync) or proxying a tool call (McpClient.CallToolAsync).
  • Ensure learn-mode flows explicitly reset IsServerCommandInvoked=false to avoid leaking a prior true value after error-driven routing into learning paths.
  • Minor modernizations/cleanup (collection expressions, simplified type references).

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
core/Microsoft.Mcp.Core/src/Areas/Server/Commands/ToolLoading/SingleProxyToolLoader.cs Applies the new IsServerCommandInvoked tagging rules across root learn/tool learn/command execution paths; minor initialization and doc cleanup.
core/Microsoft.Mcp.Core/src/Areas/Server/Commands/ToolLoading/ServerToolLoader.cs Applies consistent tag initialization and flips to true only for actual proxied tool execution; uses collection expressions.
core/Microsoft.Mcp.Core/src/Areas/Server/Commands/ToolLoading/RegistryToolLoader.cs Initializes tag to false on entry and sets true immediately before proxying the registry-discovered tool call; minor collection expression cleanup.
core/Microsoft.Mcp.Core/src/Areas/Server/Commands/ToolLoading/NamespaceToolLoader.cs Removes premature true tagging and sets true only when executing the underlying command; resets to false when entering learn paths; collection expression cleanup.
core/Microsoft.Mcp.Core/src/Areas/Server/Commands/ToolLoading/CompositeToolLoader.cs Ensures composite routing starts with IsServerCommandInvoked=false before delegating to the selected child loader.
core/Microsoft.Mcp.Core/src/Areas/Server/Commands/ToolLoading/CommandFactoryToolLoader.cs Ensures tag is false on entry and flips to true immediately before ExecuteAsync; simplifies activity tagging flow.
core/Microsoft.Mcp.Core/src/Areas/Server/Commands/ToolLoading/BaseToolLoader.cs Minor type qualification cleanup for elicitation request creation.

Comment thread core/Microsoft.Mcp.Core/src/Areas/Server/Commands/ToolLoading/SingleProxyToolLoader.cs
Comment thread core/Microsoft.Mcp.Core/src/Areas/Server/Commands/ToolLoading/CommandFactoryToolLoader.cs
Comment thread core/Microsoft.Mcp.Core/src/Areas/Server/Commands/ToolLoading/RegistryToolLoader.cs
Comment thread core/Microsoft.Mcp.Core/src/Areas/Server/Commands/ToolLoading/RegistryToolLoader.cs
conniey
conniey reviewed Feb 12, 2026
View reviewed changes
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.

Tests and changelog?

conniey
conniey reviewed Feb 13, 2026
View reviewed changes
Comment thread core/Azure.Mcp.Core/tests/Azure.Mcp.Core.UnitTests/Commands/CommandFactoryTests.cs Outdated
conniey
conniey reviewed Feb 13, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@conniey conniey left a comment •
edited
Loading

Choose a reason for hiding this comment

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

One comment

conniey
conniey reviewed Feb 13, 2026
View reviewed changes
Comment thread core/Microsoft.Mcp.Core/src/Areas/Tools/Commands/ToolsListCommand.cs Outdated
conniey
conniey reviewed Feb 13, 2026
View reviewed changes
Comment thread core/Microsoft.Mcp.Core/src/AssemblyInfo.cs Outdated
conniey
conniey approved these changes Feb 13, 2026
View reviewed changes
Comment thread core/Microsoft.Mcp.Core/src/Commands/ICommandFactory.cs
@alzimmermsft alzimmermsft merged commit a3435d2 into microsoft:main Feb 13, 2026
15 checks passed
@github-project-automation github-project-automation Bot moved this from Untriaged to Done in Azure MCP Server Feb 13, 2026
@alzimmermsft alzimmermsft deleted the InvestigateTelemetryIssues branch February 13, 2026 22:24
@alzimmermsft alzimmermsft mentioned this pull request Feb 17, 2026
Merged
20 tasks
colbytimm pushed a commit to colbytimm/microsoft-mcp that referenced this pull request Apr 20, 2026
@alzimmermsft @colbytimm
Always set IsServerCommandInvoked to false when first entering CallTo…
1b7c144 
…olHandler (microsoft#1718)

* Always set IsServerCommandInvoked to false when first entering CallToolHandler

* Changelog and some tests

* Fix small compilation issue

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

Reviewers

Copilot code review Copilot Copilot left review comments

@conniey conniey conniey approved these changes

@joshfree joshfree Awaiting requested review from joshfree joshfree is a code owner automatically assigned from microsoft/azure-mcp

@jongio jongio Awaiting requested review from jongio jongio is a code owner automatically assigned from microsoft/azure-mcp

@wbreza wbreza Awaiting requested review from wbreza wbreza is a code owner automatically assigned from microsoft/azure-mcp

@tmeschter tmeschter Awaiting requested review from tmeschter tmeschter is a code owner automatically assigned from microsoft/azure-mcp

@chidozieononiwu chidozieononiwu Awaiting requested review from chidozieononiwu chidozieononiwu is a code owner automatically assigned from microsoft/azure-mcp

@fanyang-mono fanyang-mono Awaiting requested review from fanyang-mono fanyang-mono is a code owner automatically assigned from microsoft/azure-mcp

@jairmyree jairmyree Awaiting requested review from jairmyree jairmyree is a code owner automatically assigned from microsoft/azure-mcp

Assignees

@alzimmermsft alzimmermsft

Labels

tools-Core AZMCP Core functionality that all other tools build on top of tools-Telemetry

Projects

Azure MCP Server
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@alzimmermsft @conniey