Additional Option binding updates#2922
Merged
alzimmermsft merged 8 commits intoJun 23, 2026
Merged
Conversation
…whitespace string validation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the shared option-binding system (and many downstream tool option POCOs) to separate complex/nested option handling into a new [OptionContainer] attribute, make [Option].Description required, and improve validation/error reporting during binding.
Changes:
- Introduces
[OptionContainer]for complex/nested option groups (e.g., retry policy), and removes complex-type responsibilities from[Option]. - Makes
[Option].Descriptionrequired and updates option definitions across multiple toolsets accordingly. - Improves binding-time error reporting (includes parse errors) and adds required-string empty/whitespace validation with an escape hatch (
AllowEmptyOrWhiteSpaceString), plus adjusts impacted tests/docs.
Reviewed changes
Copilot reviewed 74 out of 74 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/Fabric.Mcp.Tools.OneLake/src/Options/WorkspaceListOptions.cs | Updates [Option] usage to require Description = .... |
| tools/Fabric.Mcp.Tools.OneLake/src/Options/TableNamespaceListOptions.cs | Updates [Option] usage to require Description = .... |
| tools/Fabric.Mcp.Tools.OneLake/src/Options/TableNamespaceGetOptions.cs | Updates [Option] usage to require Description = .... |
| tools/Fabric.Mcp.Tools.OneLake/src/Options/TableListOptions.cs | Updates [Option] usage to require Description = .... |
| tools/Fabric.Mcp.Tools.OneLake/src/Options/TableGetOptions.cs | Updates [Option] usage to require Description = .... |
| tools/Fabric.Mcp.Tools.OneLake/src/Options/TableConfigGetOptions.cs | Updates [Option] usage to require Description = .... |
| tools/Fabric.Mcp.Tools.OneLake/src/Options/PathListOptions.cs | Updates [Option] usage to require Description = .... |
| tools/Fabric.Mcp.Tools.OneLake/src/Options/BlobListOptions.cs | Updates [Option] usage to require Description = .... |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/Item/OneLakeItemListDfsCommand.cs | Updates embedded options type to use required Description = .... |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/Item/OneLakeItemListCommand.cs | Updates embedded options type to use required Description = .... |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/Item/OneLakeItemDataListCommand.cs | Updates embedded options type to use required Description = .... |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/File/FileWriteCommand.cs | Updates embedded options type to use required Description = .... |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/File/FileReadCommand.cs | Updates embedded options type to use required Description = .... |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/File/FileDeleteCommand.cs | Updates embedded options type to use required Description = .... |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/File/DirectoryDeleteCommand.cs | Updates embedded options type to use required Description = .... |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/File/DirectoryCreateCommand.cs | Updates embedded options type to use required Description = .... |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/File/BlobPutCommand.cs | Updates embedded options type to use required Description = .... |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/File/BlobGetCommand.cs | Updates embedded options type to use required Description = .... |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/File/BlobDeleteCommand.cs | Updates embedded options type to use required Description = .... |
| tools/Fabric.Mcp.Tools.Docs/tests/Fabric.Mcp.Tools.Docs.Tests/Commands/GetWorkloadDefinitionCommandTests.cs | Loosens assertion to tolerate additional parse/bind error text. |
| tools/Fabric.Mcp.Tools.Docs/tests/Fabric.Mcp.Tools.Docs.Tests/Commands/GetWorkloadApisCommandTests.cs | Loosens assertion to tolerate additional parse/bind error text. |
| tools/Fabric.Mcp.Tools.Docs/tests/Fabric.Mcp.Tools.Docs.Tests/Commands/GetExamplesCommandTests.cs | Loosens assertion to tolerate additional parse/bind error text. |
| tools/Fabric.Mcp.Tools.Docs/tests/Fabric.Mcp.Tools.Docs.Tests/Commands/GetBestPracticesCommandTests.cs | Loosens assertion to tolerate additional parse/bind error text. |
| tools/Fabric.Mcp.Tools.Docs/src/Options/PublicApis/WorkloadCommandOptions.cs | Updates [Option] usage to require Description = .... |
| tools/Fabric.Mcp.Tools.Docs/src/Options/BestPractices/GetBestPracticesOptions.cs | Updates [Option] usage to require Description = .... |
| tools/Fabric.Mcp.Tools.DataFactory/src/Options/Pipeline/RunPipelineOptions.cs | Updates [Option] usage to require Description = .... |
| tools/Fabric.Mcp.Tools.DataFactory/src/Options/Pipeline/ListPipelinesOptions.cs | Updates [Option] usage to require Description = .... |
| tools/Fabric.Mcp.Tools.DataFactory/src/Options/Pipeline/GetPipelineOptions.cs | Updates [Option] usage to require Description = .... |
| tools/Fabric.Mcp.Tools.DataFactory/src/Options/Pipeline/CreatePipelineOptions.cs | Updates [Option] usage to require Description = .... |
| tools/Fabric.Mcp.Tools.DataFactory/src/Options/Dataflow/ListDataflowsOptions.cs | Updates [Option] usage to require Description = .... |
| tools/Fabric.Mcp.Tools.DataFactory/src/Options/Dataflow/ExecuteQueryOptions.cs | Updates [Option] usage to require Description = .... |
| tools/Fabric.Mcp.Tools.DataFactory/src/Options/Dataflow/CreateDataflowOptions.cs | Updates [Option] usage to require Description = .... |
| tools/Fabric.Mcp.Tools.Core/src/Options/ItemCreateOptions.cs | Updates [Option] usage to require Description = .... |
| tools/Azure.Mcp.Tools.Storage/src/Options/Table/TableListOptions.cs | Switches nested retry options to [OptionContainer] and updates descriptions. |
| tools/Azure.Mcp.Tools.Storage/src/Options/Blob/Container/ContainerGetOptions.cs | Switches nested retry options to [OptionContainer] and updates descriptions. |
| tools/Azure.Mcp.Tools.Storage/src/Options/Blob/Container/ContainerCreateOptions.cs | Switches nested retry options to [OptionContainer] and updates descriptions. |
| tools/Azure.Mcp.Tools.Storage/src/Options/Blob/BlobUploadOptions.cs | Switches nested retry options to [OptionContainer] and updates descriptions. |
| tools/Azure.Mcp.Tools.Storage/src/Options/Blob/BlobGetOptions.cs | Switches nested retry options to [OptionContainer] and updates descriptions. |
| tools/Azure.Mcp.Tools.Storage/src/Options/Account/AccountGetOptions.cs | Switches nested retry options to [OptionContainer] and updates descriptions. |
| tools/Azure.Mcp.Tools.Storage/src/Options/Account/AccountCreateOptions.cs | Switches nested retry options to [OptionContainer] and updates descriptions. |
| tools/Azure.Mcp.Tools.Kusto/tests/Azure.Mcp.Tools.Kusto.Tests/QueryCommandTests.cs | Removes unused test using import. |
| tools/Azure.Mcp.Tools.Kusto/src/Options/TableSchemaOptions.cs | Standardizes option attributes and cluster option naming (Cluster). |
| tools/Azure.Mcp.Tools.Kusto/src/Options/TableListOptions.cs | Standardizes option attributes and cluster option naming (Cluster). |
| tools/Azure.Mcp.Tools.Kusto/src/Options/SampleOptions.cs | Standardizes option attributes and cluster option naming (Cluster). |
| tools/Azure.Mcp.Tools.Kusto/src/Options/QueryOptions.cs | Standardizes option attributes and cluster option naming (Cluster). |
| tools/Azure.Mcp.Tools.Kusto/src/Options/IClusterOption.cs | Renames interface member to Cluster to match option naming. |
| tools/Azure.Mcp.Tools.Kusto/src/Options/DatabaseListOptions.cs | Standardizes option attributes and cluster option naming (Cluster). |
| tools/Azure.Mcp.Tools.Kusto/src/Options/ClusterListOptions.cs | Standardizes option attributes and retry container handling. |
| tools/Azure.Mcp.Tools.Kusto/src/Options/ClusterGetOptions.cs | Standardizes option attributes and required cluster option naming (Cluster). |
| tools/Azure.Mcp.Tools.Kusto/src/Commands/TableSchemaCommand.cs | Updates option property reference from ClusterName to Cluster. |
| tools/Azure.Mcp.Tools.Kusto/src/Commands/TableListCommand.cs | Updates option property reference from ClusterName to Cluster. |
| tools/Azure.Mcp.Tools.Kusto/src/Commands/SampleCommand.cs | Updates option property reference from ClusterName to Cluster. |
| tools/Azure.Mcp.Tools.Kusto/src/Commands/QueryCommand.cs | Updates option property reference from ClusterName to Cluster. |
| tools/Azure.Mcp.Tools.Kusto/src/Commands/DatabaseListCommand.cs | Updates option property reference from ClusterName to Cluster. |
| tools/Azure.Mcp.Tools.Kusto/src/Commands/ClusterGetCommand.cs | Updates option property reference from ClusterName to Cluster. |
| tools/Azure.Mcp.Tools.Kusto/src/Commands/BaseClusterCommand.cs | Updates validation to require Cluster instead of ClusterName. |
| tools/Azure.Mcp.Tools.ApplicationInsights/src/Options/RecommendationListOptions.cs | Switches nested retry options to [OptionContainer] and updates descriptions. |
| tools/Azure.Mcp.Tools.AppLens/src/Options/Resource/ResourceDiagnoseOptions.cs | Updates [Option] usage to require Description = .... |
| tools/Azure.Mcp.Tools.AppConfig/src/Options/KeyValue/Lock/KeyValueLockSetOptions.cs | Switches nested retry options to [OptionContainer] and updates descriptions. |
| tools/Azure.Mcp.Tools.Aks/src/Options/Nodepool/NodepoolGetOptions.cs | Switches nested retry options to [OptionContainer] and updates descriptions. |
| tools/Azure.Mcp.Tools.Aks/src/Options/Cluster/ClusterGetOptions.cs | Switches nested retry options to [OptionContainer] and updates descriptions. |
| tools/Azure.Mcp.Tools.Acr/src/Options/Registry/RegistryRepositoryListOptions.cs | Switches nested retry options to [OptionContainer] and updates descriptions. |
| tools/Azure.Mcp.Tools.Acr/src/Options/Registry/RegistryListOptions.cs | Switches nested retry options to [OptionContainer] and updates descriptions. |
| servers/Azure.Mcp.Server/docs/new-command.md | Updates option authoring guidance/examples for required Description and containers. |
| docs/option-conversion.md | Updates option conversion guidance/examples for required Description and containers. |
| core/Microsoft.Mcp.Core/tests/Microsoft.Mcp.Tests/Client/RecordedCommandTestsBase.cs | Refactors live-test-only skipping to shared helper. |
| core/Microsoft.Mcp.Core/tests/Microsoft.Mcp.Tests/Client/CommandTestsBase.cs | Ensures settings are loaded before live-test-only checks; adds helper. |
| core/Microsoft.Mcp.Core/tests/Microsoft.Mcp.Core.Tests/Options/OptionBinderTests.cs | Updates tests for required descriptions; adds new validation tests. |
| core/Microsoft.Mcp.Core/src/Options/OptionTypeHandler.cs | Adds empty/whitespace validation and routes string/enum binding accordingly. |
| core/Microsoft.Mcp.Core/src/Options/OptionDescriptor.cs | Adds [OptionContainer] support and enforces attribute correctness for complex types. |
| core/Microsoft.Mcp.Core/src/Options/OptionContainerAttribute.cs | Adds new attribute to mark complex/nested option containers. |
| core/Microsoft.Mcp.Core/src/Options/OptionBinder.cs | Includes parse errors in binding failures and simplifies validation exception creation. |
| core/Microsoft.Mcp.Core/src/Options/OptionAttribute.cs | Removes constructors and makes Description required; adds empty/whitespace allow flag. |
| core/Microsoft.Mcp.Core/src/Commands/BaseCommand`2.cs | Uses CommandValidationException.Message directly to avoid masking parser errors. |
vcolin7
reviewed
Jun 22, 2026
vcolin7
approved these changes
Jun 23, 2026
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?
[OptionContainer]attribute so[Option]doesn't perform double duty.[Option]doesn't need to support complex type handling, remove constructors and markDescriptionas a required property. Updated guidance and source code accordingly.AllowEmptyOrWhiteSpaceStringin[Option]to allow required strings to support specialty cases.GitHub issue number?
[Link to the GitHub issue this PR addresses]Pre-merge Checklist
servers/Azure.Mcp.Server/README.mdand/orservers/Fabric.Mcp.Server/README.mddocumentationREADME.mdchanges running the script./eng/scripts/Process-PackageReadMe.ps1. See Package READMEToolDescriptionEvaluatorand obtained a score of0.4or more and a top 3 ranking for all related test promptsconsolidated-tools.jsonbreaking-changelabelservers/Azure.Mcp.Server/docs/azmcp-commands.md./eng/scripts/Update-AzCommandsMetadata.ps1to update tool metadata inazmcp-commands.md(required for CI)servers/Azure.Mcp.Server/docs/e2eTestPrompts.mdcrypto mining, spam, data exfiltration, etc.)/azp run mcp - pullrequest - liveto run Live Test Pipeline