[Azure Workbooks] Refactor MCP server, fix live tests#1646
Merged
Conversation
- Fix SourceId type mismatch: convert ResourceIdentifier to string - Remove unused FuzzyMatchInfo model and ScopeResolution property - Remove unused subscriptions parameter from BuildWorkbooksQuery and BuildCountQuery - Fix HandleWorkbookException return type from object? to WorkbookError - Refactor GetWorkbooksAsync to use tuple pattern, removing dead code branch - Add documentation for static semaphore rate limiting behavior
matteing
requested review from
KarishmaGhiya,
Copilot,
jongio,
msalaman,
sandeep-sen,
vukelich,
weshaggard and
xiangyan99
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the Azure Workbooks MCP tool to support batch operations (show/delete), expands list filtering/output options, and updates unit/live tests to match the new service/command contracts.
Changes:
- Refactors
IWorkbooksService/WorkbooksServiceto add async batch APIs (GetWorkbooksAsync,DeleteWorkbooksAsync) and a richer list API returning metadata (WorkbookListResult) with filters/output modes. - Updates Workbooks commands/options to support
--workbook-ids, cross-scope listing, max-results, output format, and additional filters. - Updates unit tests and live tests to validate the new behavior and result schemas.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/Azure.Mcp.Tools.Workbooks/src/Services/WorkbooksService.cs | Implements new list/batch APIs, KQL query building, throttling, and error mapping. |
| tools/Azure.Mcp.Tools.Workbooks/src/Services/IWorkbooksService.cs | Updates the service contract to new async/list/batch operations. |
| tools/Azure.Mcp.Tools.Workbooks/src/Commands/Workbooks/ListWorkbooksCommand.cs | Switches to global command, adds new list options/filters/output handling. |
| tools/Azure.Mcp.Tools.Workbooks/src/Commands/Workbooks/ShowWorkbooksCommand.cs | Migrates show to batch --workbook-ids and returns Workbooks+Errors. |
| tools/Azure.Mcp.Tools.Workbooks/src/Commands/Workbooks/DeleteWorkbooksCommand.cs | Migrates delete to batch --workbook-ids and returns Succeeded+Errors. |
| tools/Azure.Mcp.Tools.Workbooks/src/Commands/Workbooks/UpdateWorkbooksCommand.cs | Updates to the new async update service method name. |
| tools/Azure.Mcp.Tools.Workbooks/src/Commands/Workbooks/CreateWorkbooksCommand.cs | Updates to the new async create service method name. |
| tools/Azure.Mcp.Tools.Workbooks/src/Commands/WorkbooksJsonContext.cs | Adds source-gen serializers for new result/error models and lists. |
| tools/Azure.Mcp.Tools.Workbooks/src/Options/WorkbooksOptionDefinitions.cs | Adds new CLI options (workbook-ids, filters, output-format, max-results, include-total-count). |
| tools/Azure.Mcp.Tools.Workbooks/src/Options/Workbook/ListWorkbooksOptions.cs | Adds list scopes/filters/output knobs and maps them to WorkbookFilters. |
| tools/Azure.Mcp.Tools.Workbooks/src/Options/Workbook/ShowWorkbooksOptions.cs | Switches show input from single ID to array of IDs. |
| tools/Azure.Mcp.Tools.Workbooks/src/Options/Workbook/DeleteWorkbookOptions.cs | Switches delete input from single ID to array of IDs. |
| tools/Azure.Mcp.Tools.Workbooks/src/Models/WorkbookListResult.cs | New model for list results with total count/paging metadata. |
| tools/Azure.Mcp.Tools.Workbooks/src/Models/WorkbookBatchResult.cs | New models for batch show and batch delete outcomes. |
| tools/Azure.Mcp.Tools.Workbooks/src/Models/WorkbookError.cs | New model for per-workbook errors. |
| tools/Azure.Mcp.Tools.Workbooks/src/Models/OutputFormat.cs | New enum for list output shape (Summary/Standard/Full). |
| tools/Azure.Mcp.Tools.Workbooks/src/Models/WorkbookFilters.cs | Adds semantic filters (name-contains, modified-after) and updates HasFilters. |
| tools/Azure.Mcp.Tools.Workbooks/tests/Azure.Mcp.Tools.Workbooks.UnitTests/*.cs | Updates unit tests for new service signatures and result schemas. |
| tools/Azure.Mcp.Tools.Workbooks/tests/Azure.Mcp.Tools.Workbooks.LiveTests/WorkbooksCommandTests.cs | Adds/updates live tests for list/show/delete batch behavior and new list features. |
| tools/Azure.Mcp.Tools.Workbooks/tests/Azure.Mcp.Tools.Workbooks.LiveTests/assets.json | Updates the assets tag for recorded/live test resources. |
…kbooksCommand.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…stWorkbooksCommand Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ericajlin
approved these changes
Feb 19, 2026
Contributor
Author
|
/azp run mcp - pullrequest - live |
|
Azure Pipelines successfully started running 1 pipeline(s). |
KarishmaGhiya
requested changes
Feb 24, 2026
github-project-automation
Bot
moved this from Untriaged
to In Progress
in Azure MCP Server
Address review feedback to use SubscriptionCommand instead of GlobalCommand, consistent with all other tools in the repo. - Change ListWorkbooksOptions base from GlobalOptions to SubscriptionOptions - Change ListWorkbooksCommand base from GlobalCommand to SubscriptionCommand - Remove manual subscription option registration/binding (handled by base) - Update test for required subscription validation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
|
Fixes #1144 |
…pe discovery Change ListWorkbooksCommand base class from SubscriptionCommand to GlobalCommand so subscription is optional. This enables the cross-scope discovery feature where workbooks can be listed across all accessible subscriptions when --subscription is omitted. Update unit test to verify the command succeeds without subscription instead of expecting a validation error. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…mmand Change ListWorkbooksCommand base class back to SubscriptionCommand so subscription is required. Remove duplicate subscription option registration and binding that were added when using GlobalCommand. Update unit test to expect validation error when subscription is missing. Remove live test that expected cross-scope discovery without subscription. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Author
|
/azp run mcp - pullrequest - live |
|
Azure Pipelines successfully started running 1 pipeline(s). |
KarishmaGhiya
approved these changes
Mar 3, 2026
colbytimm
pushed a commit
to colbytimm/microsoft-mcp
that referenced
this pull request
Apr 20, 2026
* Fix bugs in Workbooks toolset identified during code review - Fix SourceId type mismatch: convert ResourceIdentifier to string - Remove unused FuzzyMatchInfo model and ScopeResolution property - Remove unused subscriptions parameter from BuildWorkbooksQuery and BuildCountQuery - Fix HandleWorkbookException return type from object? to WorkbookError - Refactor GetWorkbooksAsync to use tuple pattern, removing dead code branch - Add documentation for static semaphore rate limiting behavior * Revert accidental changes to LiveTestSettingsFixture * Update tools/Azure.Mcp.Tools.Workbooks/src/Commands/Workbooks/ListWorkbooksCommand.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * review(workbooks): Review suggestions from GitHub Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix(workbooks): remove unused assignment Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix: replace HasOption/GetValueForOption with GetValueOrDefault in ListWorkbooksCommand Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: restore SubscriptionCommand base class for ListWorkbooksCommand Address review feedback to use SubscriptionCommand instead of GlobalCommand, consistent with all other tools in the repo. - Change ListWorkbooksOptions base from GlobalOptions to SubscriptionOptions - Change ListWorkbooksCommand base from GlobalCommand to SubscriptionCommand - Remove manual subscription option registration/binding (handled by base) - Update test for required subscription validation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: make subscription optional in ListWorkbooksCommand for cross-scope discovery Change ListWorkbooksCommand base class from SubscriptionCommand to GlobalCommand so subscription is optional. This enables the cross-scope discovery feature where workbooks can be listed across all accessible subscriptions when --subscription is omitted. Update unit test to verify the command succeeds without subscription instead of expecting a validation error. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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 contains several quality improvements and bug fixes to the Workbooks MCP. It also fixes live tests.
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 promptsconsolidated-tools.json/servers/Azure.Mcp.Server/docs/e2eTestPrompts.mdcrypto mining, spam, data exfiltration, etc.)/azp run mcp - pullrequest - liveto run Live Test Pipeline