Use NamespaceToolLoader for consolidated mode#1002
Merged
fanyang-mono merged 5 commits intomicrosoft:mainfrom Oct 30, 2025
Merged
Use NamespaceToolLoader for consolidated mode#1002fanyang-mono merged 5 commits intomicrosoft:mainfrom
fanyang-mono merged 5 commits intomicrosoft:mainfrom
Conversation
joshfree
added
server-Azure.Mcp
fanyang-mono
requested review from
chidozieononiwu,
conniey,
Copilot,
jongio,
joshfree,
sandeep-sen,
wbreza and
xiangyan99
fanyang-mono
changed the title
Contributor
Author
|
@conniey @anuchandy I would like to hear your feedbacks on this PR. |
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the consolidated tool discovery mechanism to separate concerns and enable direct in-process execution of consolidated commands. The key changes decouple ConsolidatedToolDiscoveryStrategy from the responsibility of creating server providers, introducing a new method CreateConsolidatedCommandFactory() that builds a dedicated CommandFactory with consolidated command groups.
- Refactored
ConsolidatedToolDiscoveryStrategyto create a newCommandFactoryinstance for consolidated tools rather than mutating the original - Added
NamespaceToolLoaderinstantiation in ConsolidatedProxy mode to enable direct in-process execution alongside proxy tools - Enhanced
NamespaceToolLoaderwith an optional filter parameter to control namespace filtering behavior
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| ConsolidatedToolDiscoveryStrategy.cs | Refactored to extract command factory creation logic and return empty provider list from DiscoverServersAsync |
| ServiceCollectionExtensions.cs | Added composite tool loader setup for ConsolidatedProxy mode combining server and namespace loaders |
| NamespaceToolLoader.cs | Added optional applyFilter parameter to control filtering behavior and propagate tool metadata hints |
| ConsolidatedToolDiscoveryStrategyTests.cs | Updated test expectations and added new tests for CreateConsolidatedCommandFactory method |
| launch.json | Added new debug configuration for consolidated mode testing |
joshfree
reviewed
Oct 30, 2025
Member
joshfree
left a comment
joshfree
left a comment
There was a problem hiding this comment.
Add a CHANGELOG entry please :)
joshfree
approved these changes
Oct 30, 2025
joshfree
approved these changes
Oct 30, 2025
Member
|
LGTM - thanks Fan |
colbytimm
pushed a commit
to colbytimm/microsoft-mcp
that referenced
this pull request
Dec 8, 2025
* Use NamespaceToolLoader for consolidated mode * Add a switch to dynamically filterring for specific namespaces or not * Add tests * Update CHANGELOG
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
This PR should resolve the following two issues:
commandFactoryofConsolidatedToolDiscoveryStrategyNamespaceToolLoaderdoesGitHub issue number?
Contributes to #849
Pre-merge Checklist
servers/Azure.Mcp.Server/CHANGELOG.mdand/orservers/Fabric.Mcp.Server/CHANGELOG.mdfor product changes (features, bug fixes, UI/UX, updated dependencies)servers/Azure.Mcp.Server/README.mdand/orservers/Fabric.Mcp.Server/README.mddocumentationeng/scripts/Process-PackageReadMe.ps1. See Package README/servers/Azure.Mcp.Server/docs/azmcp-commands.mdand/or/docs/fabric-commands.md.\eng\scripts\Update-AzCommandsMetadata.ps1to update tool metadata in azmcp-commands.md (required for CI)ToolDescriptionEvaluatorand obtained a score of0.4or more and a top 3 ranking for all related test prompts/servers/Azure.Mcp.Server/docs/e2eTestPrompts.mdcrypto mining, spam, data exfiltration, etc.)/azp run mcp - pullrequest - liveto run Live Test Pipeline