Conversation
alzimmermsft
requested review from
JasonYeMSFT,
JonathanCrd,
chidozieononiwu,
fanyang-mono,
jongio,
joshfree and
saikoumudi
There was a problem hiding this comment.
Pull request overview
Refactors Fabric OneLake tooling to reduce duplication, centralize shared command behavior, and align tool naming with the expected hyphenated convention.
Changes:
- Introduces shared base command + options types (
BaseWorkspaceCommand,BaseItemCommand,BaseWorkspaceOptions,BaseItemOptions) and migrates most OneLake commands to them. - Renames OneLake command/tool names from underscore (
_) to hyphen (-) and updates tests/docs/prompts accordingly. - Adjusts “local-only” handling for file upload/download style commands via centralized metadata/behavior.
Reviewed changes
Copilot reviewed 57 out of 57 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/Fabric.Mcp.Tools.OneLake/tests/FabricOneLakeSetupTests.cs | Updates OneLake command registration assertions for hyphenated names. |
| tools/Fabric.Mcp.Tools.OneLake/tests/Commands/Table/TableNamespaceListCommandTests.cs | Updates expected command name for hyphenated naming. |
| tools/Fabric.Mcp.Tools.OneLake/tests/Commands/Table/TableNamespaceGetCommandTests.cs | Updates expected command name for hyphenated naming. |
| tools/Fabric.Mcp.Tools.OneLake/tests/Commands/Table/TableListCommandTests.cs | Updates expected command name for hyphenated naming. |
| tools/Fabric.Mcp.Tools.OneLake/tests/Commands/Table/TableGetCommandTests.cs | Updates expected command name for hyphenated naming. |
| tools/Fabric.Mcp.Tools.OneLake/tests/Commands/Table/TableConfigGetCommandTests.cs | Updates expected command name for hyphenated naming. |
| tools/Fabric.Mcp.Tools.OneLake/tests/Commands/PathListCommandTests.cs | Updates expected command name for hyphenated naming. |
| tools/Fabric.Mcp.Tools.OneLake/tests/Commands/OneLakeWorkspaceListCommandTests.cs | Updates expected command name for hyphenated naming. |
| tools/Fabric.Mcp.Tools.OneLake/tests/Commands/OneLakeItemListCommandTests.cs | Updates expected command name for hyphenated naming. |
| tools/Fabric.Mcp.Tools.OneLake/tests/Commands/OneLakeItemDataListCommandTests.cs | Updates expected command name for hyphenated naming. |
| tools/Fabric.Mcp.Tools.OneLake/tests/Commands/DirectoryDeleteCommandTests.cs | Updates expected command name for hyphenated naming. |
| tools/Fabric.Mcp.Tools.OneLake/tests/Commands/DirectoryCreateCommandTests.cs | Updates expected command name for hyphenated naming. |
| tools/Fabric.Mcp.Tools.OneLake/tests/Commands/BlobPutCommandTests.cs | Updates expected command name for hyphenated naming. |
| tools/Fabric.Mcp.Tools.OneLake/tests/Commands/BlobGetCommandTests.cs | Updates expected command name for hyphenated naming and adds a using. |
| tools/Fabric.Mcp.Tools.OneLake/src/Services/OneLakeService.cs | Minor refactors/modern C# syntax; small perf tweak on list emptiness checks. |
| tools/Fabric.Mcp.Tools.OneLake/src/Prompts/OneLakePrompts.cs | Updates prompt tool name to hyphenated naming. |
| tools/Fabric.Mcp.Tools.OneLake/src/Options/TableNamespaceListOptions.cs | Removes now-redundant options type (replaced by base options). |
| tools/Fabric.Mcp.Tools.OneLake/src/Options/TableNamespaceGetOptions.cs | Switches to base item options + retains namespace input. |
| tools/Fabric.Mcp.Tools.OneLake/src/Options/TableListOptions.cs | Switches to base item options + retains namespace input. |
| tools/Fabric.Mcp.Tools.OneLake/src/Options/TableGetOptions.cs | Switches to base item options + retains namespace/table input. |
| tools/Fabric.Mcp.Tools.OneLake/src/Options/TableConfigGetOptions.cs | Repurposes file to define base workspace options (shared). |
| tools/Fabric.Mcp.Tools.OneLake/src/Options/PathListOptions.cs | Switches to base item options. |
| tools/Fabric.Mcp.Tools.OneLake/src/Options/OneLakeOptionDefinitions.cs | Cleans up unused imports. |
| tools/Fabric.Mcp.Tools.OneLake/src/Options/FabricOptionDefinitions.cs | Cleans up unused imports. |
| tools/Fabric.Mcp.Tools.OneLake/src/Options/BlobListOptions.cs | Switches to base item options. |
| tools/Fabric.Mcp.Tools.OneLake/src/Options/BaseItemOptions.cs | Adds shared item-scoped options. |
| tools/Fabric.Mcp.Tools.OneLake/src/Models/TableNamespaceListResult.cs | Cleans up unused imports. |
| tools/Fabric.Mcp.Tools.OneLake/src/Models/TableNamespaceGetResult.cs | Cleans up unused imports. |
| tools/Fabric.Mcp.Tools.OneLake/src/Models/TableListResult.cs | Cleans up unused imports. |
| tools/Fabric.Mcp.Tools.OneLake/src/Models/TableGetResult.cs | Cleans up unused imports. |
| tools/Fabric.Mcp.Tools.OneLake/src/Models/TableConfigurationResult.cs | Cleans up unused imports. |
| tools/Fabric.Mcp.Tools.OneLake/src/Models/OneLakeJsonContext.cs | Cleans up unused imports. |
| tools/Fabric.Mcp.Tools.OneLake/src/Models/BlobDownloadOptions.cs | Cleans up unused imports. |
| tools/Fabric.Mcp.Tools.OneLake/src/FabricOneLakeSetup.cs | Removes unused using after command reorg. |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/Workspace/OneLakeWorkspaceListCommand.cs | Renames command to hyphenated name and removes redundant usings. |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/Table/TableNamespaceListCommand.cs | Migrates to base item command + hyphenated name; removes local option wiring. |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/Table/TableNamespaceGetCommand.cs | Migrates to base item command + hyphenated name; updates option registration. |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/Table/TableListCommand.cs | Migrates to base item command + hyphenated name; updates option registration. |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/Table/TableGetCommand.cs | Migrates to base item command + hyphenated name; updates option registration. |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/Table/TableConfigGetCommand.cs | Migrates to base item command + hyphenated name; simplifies binding/validators. |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/Item/OneLakeItemListDfsCommand.cs | Migrates to base workspace command; simplifies result/options types. |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/Item/OneLakeItemListCommand.cs | Migrates to base workspace command; simplifies result/options types + hyphenated name. |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/Item/OneLakeItemDataListCommand.cs | Migrates to base workspace command; simplifies result/options types + hyphenated name. |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/File/PathListCommand.cs | Migrates to base item command + hyphenated name; removes duplicate option wiring. |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/File/FileWriteCommand.cs | Migrates to base item command; sets LocalRequired true; removes duplicate option wiring. |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/File/FileReadCommand.cs | Migrates to base item command; sets LocalRequired true; removes transport-specific checks. |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/File/FileDeleteCommand.cs | Migrates to base item command + hyphenated name; removes duplicate option wiring. |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/File/DirectoryDeleteCommand.cs | Migrates to base item command + hyphenated name; removes duplicate option wiring. |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/File/DirectoryCreateCommand.cs | Migrates to base item command + hyphenated name; simplifies result type. |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/File/BlobPutCommand.cs | Migrates to base item command + hyphenated name; tightens content/local file validation; sets LocalRequired true. |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/File/BlobListCommand.cs | Migrates to base item command; removes duplicate option wiring. |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/File/BlobGetCommand.cs | Migrates to base item command + hyphenated name; sets LocalRequired true; removes transport-specific checks. |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/File/BlobDeleteCommand.cs | Migrates to base item command; removes duplicate option wiring. |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/BaseWorkspaceCommand.cs | Adds shared workspace option registration/binding + validation. |
| tools/Fabric.Mcp.Tools.OneLake/src/Commands/BaseItemCommand.cs | Adds shared item option registration/binding + validation. |
| servers/Fabric.Mcp.Server/README.md | Updates published tool names to hyphenated naming. |
| servers/Azure.Mcp.Server/TROUBLESHOOTING.md | Partially updates OneLake tool naming examples (mixed underscore/hyphen remains). |
| command.Options.Add(FabricOptionDefinitions.Namespace.AsOptional()); | ||
| command.Options.Add(FabricOptionDefinitions.Schema.AsOptional()); |
There was a problem hiding this comment.
This command now registers --namespace as optional, but ExecuteAsync still dereferences options.Namespace! and passes it to the service, which will fail when neither --namespace nor --schema is provided. Make Namespace required again, or add a validator that requires at least one of (--namespace, --schema) and ensures options.Namespace is populated before execution.
| : options.Item; | ||
|
|
||
| var namespaceResult = await _oneLakeService.GetTableNamespaceAsync(workspaceIdentifier!, itemIdentifier!, options.Namespace!, cancellationToken); | ||
| var namespaceResult = await _oneLakeService.GetTableNamespaceAsync(options.WorkspaceId!, options.ItemId!, options.Namespace!, cancellationToken); |
There was a problem hiding this comment.
This command now registers --namespace as optional, but ExecuteAsync still dereferences options.Namespace! and passes it to the service, which will fail when neither --namespace nor --schema is provided. Make Namespace required again, or add a validator that requires at least one of (--namespace, --schema) and ensures options.Namespace is populated before execution.
| command.Options.Add(FabricOptionDefinitions.ItemId.AsOptional()); | ||
| command.Options.Add(FabricOptionDefinitions.Item.AsOptional()); | ||
| command.Options.Add(FabricOptionDefinitions.Namespace.AsRequired()); | ||
| command.Options.Add(FabricOptionDefinitions.Namespace.AsOptional()); |
There was a problem hiding this comment.
Like TableNamespaceGetCommand, --namespace is now optional but options.Namespace! is still treated as required during execution. Add a validator requiring either --namespace or --schema (and set options.Namespace accordingly), or revert Namespace to required to avoid runtime failures.
| command.Options.Add(FabricOptionDefinitions.Namespace.AsOptional()); | |
| command.Options.Add(FabricOptionDefinitions.Namespace.AsRequired()); |
| : options.Item; | ||
|
|
||
| var tablesResult = await _oneLakeService.ListTablesAsync(workspaceIdentifier!, itemIdentifier!, options.Namespace!, cancellationToken); | ||
| var tablesResult = await _oneLakeService.ListTablesAsync(options.WorkspaceId!, options.ItemId!, options.Namespace!, cancellationToken); |
There was a problem hiding this comment.
Like TableNamespaceGetCommand, --namespace is now optional but options.Namespace! is still treated as required during execution. Add a validator requiring either --namespace or --schema (and set options.Namespace accordingly), or revert Namespace to required to avoid runtime failures.
| command.Options.Add(FabricOptionDefinitions.ItemId.AsOptional()); | ||
| command.Options.Add(FabricOptionDefinitions.Item.AsOptional()); | ||
| command.Options.Add(FabricOptionDefinitions.Namespace.AsRequired()); | ||
| command.Options.Add(FabricOptionDefinitions.Namespace.AsOptional()); |
There was a problem hiding this comment.
This command still requires a namespace value to call GetTableAsync, but --namespace is now optional and there is no validator enforcing that either --namespace or the --schema alias is provided. Add validation (or make one option required) so options.Namespace cannot be null at execution time.
| command.Options.Add(FabricOptionDefinitions.Namespace.AsOptional()); | |
| command.Options.Add(FabricOptionDefinitions.Namespace.AsRequired()); |
| @@ -87,15 +61,7 @@ public override async Task<CommandResponse> ExecuteAsync(CommandContext context, | |||
| var options = BindOptions(parseResult); | |||
| try | |||
| { | |||
There was a problem hiding this comment.
This command still requires a namespace value to call GetTableAsync, but --namespace is now optional and there is no validator enforcing that either --namespace or the --schema alias is provided. Add validation (or make one option required) so options.Namespace cannot be null at execution time.
| { | |
| { | |
| if (string.IsNullOrWhiteSpace(options.Namespace)) | |
| { | |
| throw new InvalidOperationException("Either --namespace or --schema must be provided."); | |
| } |
| ### Service Principal Returns 403 for OneLake Operations in VS Code | ||
|
|
||
| When using Azure MCP Server inside VS Code with a Service Principal authenticated via Azure CLI (`az login --service-principal`), OneLake DFS operations (such as `directory_create`, `upload_file`) may return `403 Forbidden`, even though the Service Principal has the correct permissions (e.g., Workspace Admin in Microsoft Fabric). | ||
| When using Azure MCP Server inside VS Code with a Service Principal authenticated via Azure CLI (`az login --service-principal`), OneLake DFS operations (such as `directory_create`, `upload-file`) may return `403 Forbidden`, even though the Service Principal has the correct permissions (e.g., Workspace Admin in Microsoft Fabric). |
There was a problem hiding this comment.
The troubleshooting examples mix old underscore-style command names (directory_create) with the new hyphenated naming (upload-file). Update the remaining OneLake examples to the new tool/command names (and include the correct onelake_ prefix if that’s what callers use) so readers can reliably copy/paste working commands.
|
|
||
| - OneLake **blob** operations (e.g., `file_list` without a path) may succeed with `200 OK` | ||
| - OneLake **DFS** operations (e.g., `directory_create`, `upload_file`) fail with `403 Forbidden` | ||
| - OneLake **DFS** operations (e.g., `directory_create`, `upload-file`) fail with `403 Forbidden` |
There was a problem hiding this comment.
The troubleshooting examples mix old underscore-style command names (directory_create) with the new hyphenated naming (upload-file). Update the remaining OneLake examples to the new tool/command names (and include the correct onelake_ prefix if that’s what callers use) so readers can reliably copy/paste working commands.
What does this PR do?
Cleans up Fabric OneLake commands with the following:
BaseWorkspaceCommandandBaseItemCommand, and corresponding Options classes, for common functionality across most commands.stdio, but now that is managed centrally._with-, which is expected as_is meant to be the group name delimiter.GitHub issue number?
[Link to the GitHub issue this PR addresses]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.jsonbreaking-changelabel/servers/Azure.Mcp.Server/docs/e2eTestPrompts.mdcrypto mining, spam, data exfiltration, etc.)/azp run mcp - pullrequest - liveto run Live Test Pipeline