[azsdk-cli] Refactor CLI command classes and remove output helper usage#12125
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This is a large refactoring effort that modernizes the CLI command architecture by removing direct output helper usage from tools and introducing structured command response handling. The changes enable better telemetry collection and provide finer control over command execution flow.
- Wrapped CLI command functions in the base class for consistent telemetry and output handling
- Added
GetCommands()method for multi-command tools and simplified command response handling - Migrated tools to return
CommandResponseobjects instead of using direct output calls
Reviewed Changes
Copilot reviewed 91 out of 91 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tools/azsdk-cli/docs/new-tool.md | Updated documentation to reflect new MCPToolBase architecture and removed IOutputHelper references |
| tools/azsdk-cli/README.md | Updated development guidelines to remove Console API restrictions |
| Multiple tool files | Refactored to use new base classes (MCPTool/MCPMultiCommandTool) and return CommandResponse objects |
| Core architecture files | Introduced new base classes MCPToolBase, MCPTool, MCPMultiCommandTool with instrumentation |
| Response model files | Updated all response classes to inherit from CommandResponse instead of Response |
| Test files | Updated to work with new architecture and removed IOutputHelper dependencies |
| Project files | Removed Azure.Sdk.Tools.Cli.Contract project dependency |
Comments suppressed due to low confidence (2)
tools/azsdk-cli/Azure.Sdk.Tools.Cli/Tools/ReleasePlan/SpecWorkFlowTool.cs:1
- Missing comma after the first
new(SubCommandName1...)statement. This will cause a compilation error.
// Copyright (c) Microsoft Corporation.
tools/azsdk-cli/Azure.Sdk.Tools.Cli/Tools/ReleasePlan/SpecWorkFlowTool.cs:1
- The
SetHandlermethod doesn't exist onList<Command>. The handler should be set on individual commands, not on the list itself.
// Copyright (c) Microsoft Corporation.
scbedd
approved these changes
Sep 17, 2025
praveenkuttappan
approved these changes
Sep 18, 2025
Member
Author
|
Fixes #11686 |
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.
This is a somewhat large refactoring effort which adds a few key improvements:
GetCommandsto allow a tool class to return an array of sub-commands, instead of being required to construct it oneself, for example:output.Output,SetFailure()orctx.ExitCode = ExitCode. Instead it's based on theHandleCommandresponse.Additional minor improvements:
CommandResponse(FYI @praveenkuttappan)