Introduce Run command for Automation #154
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new "Run" command to the Agent 365 CLI that provides a unified, end-to-end deployment workflow for automating agent deployment, particularly designed for CI/CD pipelines. The command intelligently combines setup, deployment, and publishing steps into a single command, reducing the complexity teams face when running multiple commands to publish their agent.
Changes:
- Added new
RunCommandclass that orchestrates setup, deploy, and publish operations in a single workflow - Integrated the new command into the CLI's root command structure
- Added command name constant for the new
runcommand
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 15 comments.
| File | Description |
|---|---|
src/Microsoft.Agents.A365.DevTools.Cli/Commands/RunCommand.cs |
New 1200+ line command implementing unified deployment workflow with intelligent setup detection, deployment, and manifest publishing |
src/Microsoft.Agents.A365.DevTools.Cli/Program.cs |
Registered the RunCommand with dependency injection and added it to the root command |
src/Microsoft.Agents.A365.DevTools.Cli/Constants/CommandNames.cs |
Added constant for the new "run" command name |
Comments suppressed due to low confidence (2)
src/Microsoft.Agents.A365.DevTools.Cli/Commands/RunCommand.cs:1
- The
ExecutePublishAsyncmethod (lines 859-1105) contains significant code duplication with thePublishCommandclass. This duplicates manifest update logic, zip creation, MOS prerequisite checks, token acquisition, and upload logic. Consider extracting this shared functionality into a reusable service or helper class that both commands can use. This would improve maintainability and ensure consistency between the two commands.
// Copyright (c) Microsoft Corporation.
src/Microsoft.Agents.A365.DevTools.Cli/Commands/RunCommand.cs:1
- The
ExecutePublishAsyncmethod (lines 859-1105) contains significant code duplication with thePublishCommandclass. This duplicates manifest update logic, zip creation, MOS prerequisite checks, token acquisition, and upload logic. Consider extracting this shared functionality into a reusable service or helper class that both commands can use. This would improve maintainability and ensure consistency between the two commands.
// Copyright (c) Microsoft Corporation.
|
|
||
| // CRITICAL: Wait for file system to ensure config file is fully written | ||
| logger.LogInformation("Ensuring configuration file is synchronized..."); | ||
| await Task.Delay(2000); |
There was a problem hiding this comment.
Hard-coded 2-second delay for file system synchronization is a code smell. This indicates a race condition or timing issue that should be addressed properly. Consider implementing a polling mechanism with retry logic that checks for file existence and validity, or use proper file system synchronization primitives.
| { | ||
| logger.LogInformation("STEP 3/3: Publishing Agent Manifest..."); | ||
| logger.LogInformation("─────────────────────────────────────────"); | ||
| await ExecutePublishAsync(config, logger, configService, graphApiService, blueprintService, manifestTemplateService); |
There was a problem hiding this comment.
The ExecutePublishAsync method (lines 859-1105) contains significant code duplication with the PublishCommand class. This duplicates manifest update logic, zip creation, MOS prerequisite checks, token acquisition, and upload logic. Consider extracting this shared functionality into a reusable service or helper class that both commands can use. This would improve maintainability and ensure consistency between the two commands.
| // Create manifest.zip | ||
| var zipPath = Path.Combine(manifestDir, "manifest.zip"); | ||
| if (File.Exists(zipPath)) | ||
| { | ||
| try { File.Delete(zipPath); } catch { /* ignore */ } | ||
| } | ||
|
|
||
| var expectedFiles = new List<string>(); | ||
| string[] candidateNames = ["manifest.json", "color.png", "outline.png", "logo.png", "icon.png"]; | ||
| foreach (var name in candidateNames) | ||
| { | ||
| var p = Path.Combine(manifestDir, name); | ||
| if (File.Exists(p)) expectedFiles.Add(p); | ||
| if (expectedFiles.Count == 4) break; | ||
| } | ||
|
|
||
| if (expectedFiles.Count < 4) |
There was a problem hiding this comment.
The ExecutePublishAsync method (lines 859-1105) contains significant code duplication with the PublishCommand class. This duplicates manifest update logic, zip creation, MOS prerequisite checks, token acquisition, and upload logic. Consider extracting this shared functionality into a reusable service or helper class that both commands can use. This would improve maintainability and ensure consistency between the two commands.
| // Create manifest.zip | |
| var zipPath = Path.Combine(manifestDir, "manifest.zip"); | |
| if (File.Exists(zipPath)) | |
| { | |
| try { File.Delete(zipPath); } catch { /* ignore */ } | |
| } | |
| var expectedFiles = new List<string>(); | |
| string[] candidateNames = ["manifest.json", "color.png", "outline.png", "logo.png", "icon.png"]; | |
| foreach (var name in candidateNames) | |
| { | |
| var p = Path.Combine(manifestDir, name); | |
| if (File.Exists(p)) expectedFiles.Add(p); | |
| if (expectedFiles.Count == 4) break; | |
| } | |
| if (expectedFiles.Count < 4) | |
| // Create manifest.zip and collect expected files | |
| (var zipPath, var expectedFiles) = CreateManifestZip(manifestDir); | |
| { | |
| try | |
| { | |
| File.Delete(zipFilePath); | |
| } | |
| catch | |
| { | |
| // Ignore IO exceptions when deleting existing manifest.zip | |
| } | |
| } | |
| if (files.Count == 4) | |
| { | |
| break; | |
| } | |
| } | |
| return (zipFilePath, files); | |
| } |
| using (var zipStream = new FileStream(zipPath, FileMode.Create, FileAccess.ReadWrite)) | ||
| using (var archive = new ZipArchive(zipStream, ZipArchiveMode.Create)) |
There was a problem hiding this comment.
The ExecutePublishAsync method (lines 859-1105) contains significant code duplication with the PublishCommand class. This duplicates manifest update logic, zip creation, MOS prerequisite checks, token acquisition, and upload logic. Consider extracting this shared functionality into a reusable service or helper class that both commands can use. This would improve maintainability and ensure consistency between the two commands.
| try | ||
| { | ||
| logger.LogDebug("Checking MOS prerequisites (service principals and permissions)..."); | ||
| var mosPrereqsConfigured = await PublishHelpers.EnsureMosPrerequisitesAsync( |
There was a problem hiding this comment.
The ExecutePublishAsync method (lines 859-1105) contains significant code duplication with the PublishCommand class. This duplicates manifest update logic, zip creation, MOS prerequisite checks, token acquisition, and upload logic. Consider extracting this shared functionality into a reusable service or helper class that both commands can use. This would improve maintainability and ensure consistency between the two commands.
| /// <summary> | ||
| /// Updates manifest.json with the blueprint ID and display name. | ||
| /// </summary> | ||
| private static async Task<string> UpdateManifestFileAsync( |
There was a problem hiding this comment.
The manifest update methods UpdateManifestFileAsync and UpdateAgenticUserManifestTemplateFileAsync are duplicated from PublishCommand. Extract these to a shared helper or service class to avoid duplication.
| /// <summary> | ||
| /// Updates agenticUserTemplateManifest.json with the blueprint ID. | ||
| /// </summary> | ||
| private static async Task<string> UpdateAgenticUserManifestTemplateFileAsync( |
There was a problem hiding this comment.
The manifest update methods UpdateManifestFileAsync and UpdateAgenticUserManifestTemplateFileAsync are duplicated from PublishCommand. Extract these to a shared helper or service class to avoid duplication.
| /// <summary> | ||
| /// Executes application deployment to Azure Web App. | ||
| /// </summary> | ||
| private static async Task ExecuteDeployAsync( |
There was a problem hiding this comment.
The new RunCommand class lacks test coverage. Given that the repository has comprehensive test coverage for other commands (DeployCommand, PublishCommand, SetupCommand, etc.), test coverage should be added for the RunCommand, particularly for the complex workflow orchestration logic and error handling paths.
| foreach (var name in candidateNames) | ||
| { | ||
| var p = Path.Combine(manifestDir, name); | ||
| if (File.Exists(p)) expectedFiles.Add(p); | ||
| if (expectedFiles.Count == 4) break; | ||
| } |
There was a problem hiding this comment.
This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.
| skipInfrastructure, | ||
| CancellationToken.None); | ||
|
|
||
| setupResults.InfrastructureCreated = skipInfrastructure ? false : setupInfra; |
There was a problem hiding this comment.
The expression 'A ? false : B' can be simplified to '!A && B'.
| setupResults.InfrastructureCreated = skipInfrastructure ? false : setupInfra; | |
| setupResults.InfrastructureCreated = !skipInfrastructure && setupInfra; |
Teams face issues to run multiple commands to publish their agent. Run commands takes a comprehensive approach specially for CI/CD pipelines and automation to create resources, deploy agent and publish.
This is the first iteration of the command and we'll improve the experience as we go.