Skip to content

Add Deploy mcp sub commands#7

Merged
mengyimicro merged 2 commits into
mainfrom
users/mengyixu/AddDeployMcpCommands
Nov 14, 2025
Merged

Add Deploy mcp sub commands#7
mengyimicro merged 2 commits into
mainfrom
users/mengyixu/AddDeployMcpCommands

Conversation

@mengyimicro
Copy link
Copy Markdown
Contributor

Add Deploy MCP subcommands and refactor deployment architecture

This commit introduces a modular deployment system for the Agent365 DevTools CLI
with separate subcommands for application and MCP tool permission deployment.

Key Features Added:

New Subcommands:

  • a365 deploy app - Deploy only application binaries to Azure App Service
  • a365 deploy mcp - Deploy/update only MCP server scopes and permissions
  • Enhanced main a365 deploy command now performs both operations sequentially

Architecture Improvements:

  • Refactored duplicate code into reusable helper methods:
    • ValidateDeploymentPrerequisitesAsync() - Common config/Azure validation
    • DeployApplicationAsync() - Application deployment logic
    • DeployMcpToolPermissionsAsync() - MCP permissions management
    • HandleDeploymentException() - Standardized error handling

MCP Integration:

  • Added OAuth2 permission grants management
  • Implemented inheritable permissions configuration
  • Added admin consent automation for agent identities
  • Integrated with toolingManifest.json for scope requirements

Enhanced User Experience:

  • Improved dry-run output with step-by-step preview
  • Better error messages and user guidance
  • Consistent logging and progress reporting across all deployment modes
  • Maintained backward compatibility with existing deployment workflows

Code Quality:

  • Eliminated ~60 lines of duplicated code
  • Improved maintainability and testability
  • Applied DRY principle throughout deployment logic
  • Enhanced error handling consistency

Copilot AI review requested due to automatic review settings November 14, 2025 06:07
@mengyimicro mengyimicro requested a review from a team as a code owner November 14, 2025 06:07
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a modular deployment system for the Agent365 DevTools CLI with separate subcommands for application and MCP tool permission deployment. The changes add a365 deploy app and a365 deploy mcp subcommands while maintaining backward compatibility with the main a365 deploy command that runs both operations sequentially.

Key Changes:

  • Added new GraphDeleteAsync and ReplaceOauth2PermissionGrantAsync methods to GraphApiService for OAuth2 permission management
  • Refactored DeployCommand to extract reusable helper methods (ValidateDeploymentPrerequisitesAsync, DeployApplicationAsync, DeployMcpToolPermissionsAsync)
  • Standardized logging bullet points from to - across multiple files for consistency

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs Added GraphDeleteAsync and ReplaceOauth2PermissionGrantAsync methods for OAuth2 permission grant management with delete-and-recreate semantics
src/Microsoft.Agents.A365.DevTools.Cli/Commands/DeployCommand.cs Refactored into main command and two subcommands (app, mcp) with extracted helper methods for deployment validation and execution
src/Microsoft.Agents.A365.DevTools.Cli/Services/A365SetupRunner.cs Updated logging bullet points from to - for consistency
src/Microsoft.Agents.A365.DevTools.Cli/Services/A365CreateInstanceRunner.cs Updated logging bullet points from to - and refined variable naming from "agent identity" to "agentic app"
src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupCommand.cs Updated logging bullet points from to - for consistency
src/Microsoft.Agents.A365.DevTools.Cli/Commands/CreateInstanceCommand.cs Updated comments and variable names to use "agentic app" terminology consistently
docs/commands/deploy.md Added comprehensive documentation for the deploy command and its new subcommands
docs/commands/config-init.md Updated example to use a365 deploy app instead of a365 deploy
README.md Added documentation for new a365 deploy app and a365 deploy mcp subcommands

Comment thread docs/commands/deploy.md
Comment thread src/Microsoft.Agents.A365.DevTools.Cli/Commands/CreateInstanceCommand.cs Outdated
Comment thread src/Microsoft.Agents.A365.DevTools.Cli/Commands/DeployCommand.cs Outdated
Comment thread src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs
Comment thread docs/commands/deploy.md Outdated
Comment thread docs/commands/deploy.md Outdated
Comment thread src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs Outdated
Comment thread src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs Outdated
Comment thread src/Microsoft.Agents.A365.DevTools.Cli/Commands/CreateInstanceCommand.cs Outdated
Comment thread src/Microsoft.Agents.A365.DevTools.Cli/Commands/DeployCommand.cs
Comment thread docs/commands/deploy.md Outdated
Comment thread src/Microsoft.Agents.A365.DevTools.Cli/Commands/CreateInstanceCommand.cs Outdated
Comment thread src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs Outdated
Comment thread src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs
sellakumaran
sellakumaran previously approved these changes Nov 14, 2025
@mengyimicro mengyimicro merged commit 14d882a into main Nov 14, 2025
2 checks passed
@pontemonti pontemonti deleted the users/mengyixu/AddDeployMcpCommands branch December 5, 2025 17:27
sellakumaran pushed a commit that referenced this pull request Feb 27, 2026
* Add Deploy mcp sub commands

* Resolving comments
sellakumaran added a commit that referenced this pull request Mar 8, 2026
…aph, tests)

Exit codes (#7, #8/#9):
- Set Environment.ExitCode = 1 in ValidateDeploymentPrerequisitesAsync before
  each null return so callers exit non-zero on config/Web App validation failure
- Replace deploy-mcp guard `return` with ExceptionHandler.ExitWithCleanup(1)
  for AgentBlueprintId, AgenticAppId, and TenantId missing-config cases

Log severity (#15, #16, #17):
- LogCheckWarning: LogInformation -> LogWarning
- LogCheckFailure: all three LogInformation -> LogError
- ExecuteCheckWithLoggingAsync warning path: log ErrorMessage ?? Details
  so the primary warning message is no longer silently dropped

skip-graph regressions (#21, #22):
- Guard RunChecksOrExitAsync(MOS checks) behind if (!skipGraph)
- Guard clientAppId null check behind !skipGraph in PublishCommand

Unused parameter (#14):
- Remove IPrerequisiteRunner from BlueprintSubcommand.CreateCommand signature
- Update SetupCommand.cs call site and BlueprintSubcommandTests accordingly

InfrastructureRequirementCheck (#5, #6):
- Add I1/I2/I3/I1V2/I2V2/I3V2 (Isolated) SKUs to validation error message
- Wrap CheckAsync with ExecuteCheckWithLoggingAsync so [PASS]/[FAIL] is printed

PrerequisiteRunner warning message (#3):
- Log ErrorMessage ?? Details, log even when both are empty

IsCaeError gap (#18):
- Add InvalidAuthenticationToken to IsCaeError in ClientAppValidator

Stale comment (#10):
- Update ValidateDeploymentPrerequisitesAsync doc to remove "environment"

Tests (#19, #20):
- Add AppServiceAuthRequirementCheckTests (success, failure, metadata, null guard)
- Add MosPrerequisitesRequirementCheckTests (exception->failure, metadata, null guards)
- Update FrontierPreviewRequirementCheckTests: [WARN] now at LogWarning not LogInformation

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
sellakumaran added a commit that referenced this pull request Mar 10, 2026
…siteRunner (#106) (#312)

* feat: unify prerequisite validation via IRequirementCheck + IPrerequisiteRunner (#106)

Commands now declare prerequisites using IRequirementCheck and fail early
with actionable messages before any side effects occur.

Phase 1 - pure reorganization (zero behavioral change):
- Add AzureAuthRequirementCheck and InfrastructureRequirementCheck adapters
- Add IPrerequisiteRunner / PrerequisiteRunner to run checks in order
- Route AllSubcommand, BlueprintSubcommand, InfrastructureSubcommand,
  and DeployCommand through the shared runner instead of ad-hoc validators
- Delete dead code: ISubCommand.ValidateAsync, IAzureValidator/AzureValidator
- Make AzureAuthValidator.ValidateAuthenticationAsync virtual for testability

Phase 2 - minimal early-fail additions:
- cleanup azure: auth check before preview display
- deploy mcp: explicit early guards for agentBlueprintId and agenticAppId
  before any Graph/network calls

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: three CLI polish fixes

- ConfigFileNotFoundException now extends Agent365Exception so missing
  config errors surface as clean user messages (no stack trace) on all
  commands, not just those with local catch blocks. Removes ad-hoc
  FileNotFoundException catches in CleanupCommand and CreateInstanceCommand.

- config init: expand relative/dot deployment paths to absolute before
  saving so the stored value is portable across directories. Update help
  text to clarify relative paths are accepted.

- config init: drop platform-specific parenthetical from 'Allow public
  client flows' log message -- the setting is required on all platforms.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Polish CLI output: reduce noise, fix ordering, add TraceId

- Move "Running all setup steps..." to after requirements check output
- Remove redundant "Agent 365 Setup" header (user already knows the command)
- Change CorrelationId log to LogDebug for setup all and blueprint; surface
  as TraceId inline on the action line ("Running all setup steps... (TraceId: ...)")
  so it is always captured in setup.log as [INF] and visible on console
- Demote PlatformDetector internal logs to LogDebug; single "Detected project
  platform: X" line remains as the user-facing output
- Add AzureAuthRequirementCheck to GetConfigRequirementChecks so Azure auth
  appears in requirements output for all setup subcommands
- Remove redundant mid-execution auth gate from BlueprintSubcommand that caused
  duplicate [PASS] Azure Authentication output
- Fix RequirementCheck base class: use LogInformation for all check result lines
  to avoid WARNING:/ERROR: prefix doubling from logger formatter
- Collapse verbose requirements summary to single line:
  "Requirements: X passed, Y warnings, Z failed"
- Update tests to match new message text and log level assertions

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: add fail-early requirement checks to remaining commands

Extends fail-early validation to setup infrastructure, setup permissions,
setup copilot-studio, cleanup azure, deploy, and publish commands.
Each command now runs targeted IRequirementCheck-based pre-flight checks
with formatted [PASS]/[FAIL] output before executing destructive or
slow operations, surfacing auth and config failures immediately.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* refactor: structured requirement check composition + fix CAE token revocation UX

Phase 1 (zero behavioral change):
- Add GetBaseChecks() to SetupCommand and CleanupCommand for explicit check composition
- Add GetChecks() to each setup subcommand so check lists are co-located with their command
- Add RunChecksOrExitAsync() helper to RequirementsSubcommand to eliminate four-line boilerplate
- Guard all requirement check calls with if (!dryRun) to avoid spurious network calls
- Update RequirementsSubcommandTests to use public API after making internal helpers private

Fix CAE token revocation UX:
- Add ClientAppValidationException.TokenRevoked() factory for clear re-auth guidance
- Detect server-side CAE token revocation in GetClientAppInfoAsync and throw TokenRevoked
  instead of returning null (which was misreported as "app not found")
- Pass suppressErrorLogging: true to all az CLI calls in ClientAppValidator so raw error
  output no longer leaks to console before the formatted [FAIL] message
- Update ClientAppValidatorTests mocks to match suppressErrorLogging parameter

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: suppress raw subprocess output leaking before structured check results

AzureAuthValidator: add suppressErrorLogging to az account show call to
prevent CommandExecutor from printing raw stderr before [FAIL] output.
Remove verbose LogError/LogInformation guidance blocks — the validator
returns bool only; issue/resolution messaging belongs in the check layer.

PowerShellModulesRequirementCheck: downgrade auto-install progress from
LogInformation/LogWarning to LogDebug so they don't print before [PASS].

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: add cleanup azure --dry-run, AppService/MOS checks, update docs

- Add `--dry-run` flag to `a365 cleanup azure`: previews resources that
  would be deleted without requiring Azure auth or making any changes
- Add `AppServiceAuthRequirementCheck`: validates App Service deployment
  token before `a365 deploy`, catching AADSTS50173 token revocation early
- Add `MosPrerequisitesRequirementCheck`: validates MOS service principals
  before `a365 publish` proceeds, converting SetupValidationException to
  structured failure output
- Wire new checks into DeployCommand and PublishCommand via
  RunChecksOrExitAsync, replacing ad-hoc inline validation
- Add `GetChecks(AzureAuthValidator)` to InfrastructureSubcommand for
  explicit check composition
- Add `GetAppServiceTokenAsync` to AzureAuthValidator
- Update CLI design.md: add Requirements/ to project structure and
  document the IRequirementCheck prerequisite validation pattern
- Update CHANGELOG.md with user-visible additions

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: address PR #312 review comments (exit codes, log levels, skip-graph, tests)

Exit codes (#7, #8/#9):
- Set Environment.ExitCode = 1 in ValidateDeploymentPrerequisitesAsync before
  each null return so callers exit non-zero on config/Web App validation failure
- Replace deploy-mcp guard `return` with ExceptionHandler.ExitWithCleanup(1)
  for AgentBlueprintId, AgenticAppId, and TenantId missing-config cases

Log severity (#15, #16, #17):
- LogCheckWarning: LogInformation -> LogWarning
- LogCheckFailure: all three LogInformation -> LogError
- ExecuteCheckWithLoggingAsync warning path: log ErrorMessage ?? Details
  so the primary warning message is no longer silently dropped

skip-graph regressions (#21, #22):
- Guard RunChecksOrExitAsync(MOS checks) behind if (!skipGraph)
- Guard clientAppId null check behind !skipGraph in PublishCommand

Unused parameter (#14):
- Remove IPrerequisiteRunner from BlueprintSubcommand.CreateCommand signature
- Update SetupCommand.cs call site and BlueprintSubcommandTests accordingly

InfrastructureRequirementCheck (#5, #6):
- Add I1/I2/I3/I1V2/I2V2/I3V2 (Isolated) SKUs to validation error message
- Wrap CheckAsync with ExecuteCheckWithLoggingAsync so [PASS]/[FAIL] is printed

PrerequisiteRunner warning message (#3):
- Log ErrorMessage ?? Details, log even when both are empty

IsCaeError gap (#18):
- Add InvalidAuthenticationToken to IsCaeError in ClientAppValidator

Stale comment (#10):
- Update ValidateDeploymentPrerequisitesAsync doc to remove "environment"

Tests (#19, #20):
- Add AppServiceAuthRequirementCheckTests (success, failure, metadata, null guard)
- Add MosPrerequisitesRequirementCheckTests (exception->failure, metadata, null guards)
- Update FrontierPreviewRequirementCheckTests: [WARN] now at LogWarning not LogInformation

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
sellakumaran added a commit that referenced this pull request Mar 11, 2026
- Delegate zip creation to ManifestTemplateService.CreateManifestZipAsync,
  removing duplicate inline implementation and aligning file list with the
  service's canonical set (fixes comments #1 and #2)
- Move per-file zip log entries to LogDebug in ManifestTemplateService so
  the command output remains terse
- Fix --dry-run option description: "without writing files or creating the zip"
  (fixes comment #3)
- Handle null/empty displayName in name.short guidance with explicit warning
  instead of printing currently: "" (fixes comment #4)
- Update test class summary to reflect current Console.In redirect approach
  (fixes comment #6)
- Fix CHANGELOG: interactive prompts occur only in interactive terminals;
  stdin redirect suppresses them in scripts (fixes comment #7)
- Comment #5 (PowerPlatformConstants summary) already resolved by prior rename

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
sellakumaran added a commit that referenced this pull request Mar 11, 2026
… instructions (#315)

* fix: strip publish command to zip-only and remove internal MOS constants

- PublishCommand now only updates manifest files, creates manifest.zip,
  and prints manual upload instructions (Agents > All agents > Upload custom agent)
- Removed MOS Titles upload, token acquisition, and Graph API steps from publish
- Removed --mos-env, --mos-token, --skip-graph options
- Deleted AgentPublishService, MosTokenService, PublishHelpers (dead code)
- Stripped MosConstants to PowerPlatform-only; removed internal TPS/MOS Titles
  app IDs, scope GUIDs, and service URLs
- Removed dead MOS error message helpers from ErrorMessages
- Updated PublishCommandTests to match new simplified signature and added
  zip creation test

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* refactor: address code review issues in publish command cleanup

- Remove unused agentBlueprintService parameter from CreateCommand signature
  and its callsite in Program.cs (CR-001)
- Replace magic-number zip cap (4) and arbitrary file padding loop with a
  clean LINQ expression over known candidate filenames (CR-002)
- Remove dead --verbose option binding in publish command handler; startup-
  level --verbose in Program.cs continues to work (CR-003)
- Add CHANGELOG [Unreleased] entry for the breaking behavior change (CR-004)
- Remove redundant Console.SetIn call inside zip creation test; constructor
  already sets it (CR-005)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: resolve post-merge build errors from main refactoring

- Update Program.cs to use AzureAuthValidator directly (replaces removed
  IAzureValidator/AzureValidator), remove AzureWebAppCreator resolution,
  and pass authValidator to CleanupCommand
- Delete MosPrerequisitesRequirementCheck and its test — this check wraps
  the deleted PublishHelpers and is no longer needed after the publish
  command was stripped to zip-only

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* refactor: clean up publish command output to follow az cli conventions

- Replace verbose section headers and wall-of-text guidance with a terse
  column-aligned field list (version, name.short, name.full, descriptions,
  developer.*, icons)
- Remove duplicate success messages; single 'Manifest updated: {path}' line
- Move per-file zip log entries to LogDebug
- Remove mixed Console.WriteLine / logger calls; output consistent through logger
- Add blank line after interactive prompt block before 'Package created:'
- Fix column alignment: replace 'color.png / outline.png' key with 'icons'
- Add name.short > 30 char warning
- Remove unused logger parameters from UpdateManifestFileAsync and
  UpdateAgenticUserManifestTemplateFileAsync
- Update tests: remove Console.SetIn workarounds that are no longer needed
  for early-exit paths; add WithDisplayNameExceeding30Chars_LogsWarning test

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* rename: MosConstants -> PowerPlatformConstants

File and class name no longer reflect their contents after MOS upload
infrastructure was removed. The remaining constants relate solely to the
Power Platform API (CopilotStudio permissions), so rename to match.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: address PR #315 review comments

- Delegate zip creation to ManifestTemplateService.CreateManifestZipAsync,
  removing duplicate inline implementation and aligning file list with the
  service's canonical set (fixes comments #1 and #2)
- Move per-file zip log entries to LogDebug in ManifestTemplateService so
  the command output remains terse
- Fix --dry-run option description: "without writing files or creating the zip"
  (fixes comment #3)
- Handle null/empty displayName in name.short guidance with explicit warning
  instead of printing currently: "" (fixes comment #4)
- Update test class summary to reflect current Console.In redirect approach
  (fixes comment #6)
- Fix CHANGELOG: interactive prompts occur only in interactive terminals;
  stdin redirect suppresses them in scripts (fixes comment #7)
- Comment #5 (PowerPlatformConstants summary) already resolved by prior rename

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants