From 9fefa417831c866749842a1ab4df1888c1ebd330 Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Fri, 6 Mar 2026 15:35:27 -0800 Subject: [PATCH 1/3] 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 --- .../Commands/CleanupCommand.cs | 15 +- .../Commands/CreateInstanceCommand.cs | 8 +- .../Commands/DeployCommand.cs | 34 ++- .../Commands/SetupCommand.cs | 12 +- .../SetupSubcommands/AllSubcommand.cs | 63 ++--- .../SetupSubcommands/BlueprintSubcommand.cs | 65 +---- .../InfrastructureSubcommand.cs | 85 +----- .../Program.cs | 18 +- .../Services/AzureAuthValidator.cs | 2 +- .../Services/AzureValidator.cs | 54 ---- .../Services/IPrerequisiteRunner.cs | 30 +++ .../Services/ISubCommand.cs | 22 -- .../Services/PrerequisiteRunner.cs | 46 ++++ .../AzureAuthRequirementCheck.cs | 50 ++++ .../InfrastructureRequirementCheck.cs | 93 +++++++ .../Commands/BlueprintSubcommandTests.cs | 112 +++++--- .../CleanupCommandBotEndpointTests.cs | 25 +- .../Commands/CleanupCommandTests.cs | 68 +++-- .../Commands/CreateInstanceCommandTests.cs | 16 +- .../Commands/DeployCommandTests.cs | 16 +- .../Commands/SetupCommandTests.cs | 68 +++-- .../Commands/SubcommandValidationTests.cs | 110 +++----- .../Services/PrerequisiteRunnerTests.cs | 166 ++++++++++++ .../AzureAuthRequirementCheckTests.cs | 131 ++++++++++ .../InfrastructureRequirementCheckTests.cs | 245 ++++++++++++++++++ 25 files changed, 1095 insertions(+), 459 deletions(-) delete mode 100644 src/Microsoft.Agents.A365.DevTools.Cli/Services/AzureValidator.cs create mode 100644 src/Microsoft.Agents.A365.DevTools.Cli/Services/IPrerequisiteRunner.cs delete mode 100644 src/Microsoft.Agents.A365.DevTools.Cli/Services/ISubCommand.cs create mode 100644 src/Microsoft.Agents.A365.DevTools.Cli/Services/PrerequisiteRunner.cs create mode 100644 src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/AzureAuthRequirementCheck.cs create mode 100644 src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/InfrastructureRequirementCheck.cs create mode 100644 src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/PrerequisiteRunnerTests.cs create mode 100644 src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Requirements/AzureAuthRequirementCheckTests.cs create mode 100644 src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Requirements/InfrastructureRequirementCheckTests.cs diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs index 6def19ba..d1d3ac63 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs @@ -8,6 +8,7 @@ using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; using Microsoft.Agents.A365.DevTools.Cli.Services; using Microsoft.Agents.A365.DevTools.Cli.Services.Internal; +using Microsoft.Agents.A365.DevTools.Cli.Services.Requirements.RequirementChecks; using Microsoft.Agents.A365.DevTools.Cli.Models; namespace Microsoft.Agents.A365.DevTools.Cli.Commands; @@ -24,7 +25,9 @@ public static Command CreateCommand( CommandExecutor executor, AgentBlueprintService agentBlueprintService, IConfirmationProvider confirmationProvider, - FederatedCredentialService federatedCredentialService) + FederatedCredentialService federatedCredentialService, + IPrerequisiteRunner prerequisiteRunner, + AzureAuthValidator authValidator) { var cleanupCommand = new Command("cleanup", "Clean up ALL resources (blueprint, instance, Azure) - use subcommands for granular cleanup"); @@ -55,7 +58,7 @@ public static Command CreateCommand( // Add subcommands for granular control cleanupCommand.AddCommand(CreateBlueprintCleanupCommand(logger, configService, botConfigurator, executor, agentBlueprintService, confirmationProvider, federatedCredentialService)); - cleanupCommand.AddCommand(CreateAzureCleanupCommand(logger, configService, executor)); + cleanupCommand.AddCommand(CreateAzureCleanupCommand(logger, configService, executor, prerequisiteRunner, authValidator)); cleanupCommand.AddCommand(CreateInstanceCleanupCommand(logger, configService, executor)); return cleanupCommand; @@ -304,7 +307,9 @@ private static Command CreateBlueprintCleanupCommand( private static Command CreateAzureCleanupCommand( ILogger logger, IConfigService configService, - CommandExecutor executor) + CommandExecutor executor, + IPrerequisiteRunner prerequisiteRunner, + AzureAuthValidator authValidator) { var command = new Command("azure", "Remove Azure resources (App Service, App Service Plan)"); @@ -331,6 +336,10 @@ private static Command CreateAzureCleanupCommand( var config = await LoadConfigAsync(configFile, logger, configService); if (config == null) return; + var authChecks = new List { new AzureAuthRequirementCheck(authValidator) }; + if (!await prerequisiteRunner.RunAsync(authChecks, config, logger, CancellationToken.None)) + return; + logger.LogInformation(""); logger.LogInformation("Azure Cleanup Preview:"); logger.LogInformation("========================="); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CreateInstanceCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CreateInstanceCommand.cs index bb03a1e0..bd44e63b 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CreateInstanceCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CreateInstanceCommand.cs @@ -18,7 +18,7 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Commands; public class CreateInstanceCommand { public static Command CreateCommand(ILogger logger, IConfigService configService, CommandExecutor executor, - IBotConfigurator botConfigurator, GraphApiService graphApiService, IAzureValidator azureValidator) + IBotConfigurator botConfigurator, GraphApiService graphApiService) { // Command description - deprecated // Old: Create and configure agent user identities with appropriate @@ -75,12 +75,6 @@ public static Command CreateCommand(ILogger logger, IConf var instanceConfig = await LoadConfigAsync(logger, configService, config.FullName); if (instanceConfig == null) Environment.Exit(1); - // Validate Azure CLI authentication, subscription, and environment - if (!await azureValidator.ValidateAllAsync(instanceConfig.SubscriptionId)) - { - logger.LogError("Instance creation cannot proceed without proper Azure CLI authentication and subscription"); - Environment.Exit(1); - } logger.LogInformation(""); // Step 1-3: Identity, Licenses, and MCP Registration diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DeployCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DeployCommand.cs index 62cf2a83..be8bb2fd 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DeployCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DeployCommand.cs @@ -7,6 +7,7 @@ using Microsoft.Agents.A365.DevTools.Cli.Models; using Microsoft.Agents.A365.DevTools.Cli.Services; using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; +using Microsoft.Agents.A365.DevTools.Cli.Services.Requirements.RequirementChecks; using Microsoft.Extensions.Logging; using System.CommandLine; @@ -19,7 +20,8 @@ public static Command CreateCommand( IConfigService configService, CommandExecutor executor, DeploymentService deploymentService, - IAzureValidator azureValidator, + IPrerequisiteRunner prerequisiteRunner, + AzureAuthValidator authValidator, GraphApiService graphApiService, AgentBlueprintService blueprintService) { @@ -53,7 +55,7 @@ public static Command CreateCommand( command.AddOption(restartOption); // Add subcommands - command.AddCommand(CreateAppSubcommand(logger, configService, executor, deploymentService, azureValidator)); + command.AddCommand(CreateAppSubcommand(logger, configService, executor, deploymentService, prerequisiteRunner, authValidator)); command.AddCommand(CreateMcpSubcommand(logger, configService, executor, graphApiService, blueprintService)); // Single handler for the deploy command - runs only the application deployment flow @@ -82,7 +84,7 @@ public static Command CreateCommand( } var validatedConfig = await ValidateDeploymentPrerequisitesAsync( - config.FullName, configService, azureValidator, executor, logger); + config.FullName, configService, prerequisiteRunner, authValidator, executor, logger); if (validatedConfig == null) return; await DeployApplicationAsync(validatedConfig, deploymentService, verbose, inspect, restart, logger); @@ -101,7 +103,8 @@ private static Command CreateAppSubcommand( IConfigService configService, CommandExecutor executor, DeploymentService deploymentService, - IAzureValidator azureValidator) + IPrerequisiteRunner prerequisiteRunner, + AzureAuthValidator authValidator) { var command = new Command("app", "Deploy Microsoft Agent 365 application binaries to the configured Azure App Service"); @@ -157,7 +160,7 @@ private static Command CreateAppSubcommand( } var validatedConfig = await ValidateDeploymentPrerequisitesAsync( - config.FullName, configService, azureValidator, executor, logger); + config.FullName, configService, prerequisiteRunner, authValidator, executor, logger); if (validatedConfig == null) return; await DeployApplicationAsync(validatedConfig, deploymentService, verbose, inspect, restart, logger); @@ -218,6 +221,18 @@ private static Command CreateMcpSubcommand( var updateConfig = await configService.LoadAsync(config.FullName); if (updateConfig == null) Environment.Exit(1); + // Early validation: fail before any network calls + if (string.IsNullOrWhiteSpace(updateConfig.AgentBlueprintId)) + { + logger.LogError("agentBlueprintId is not configured. Run 'a365 setup all' to create the agent blueprint."); + return; + } + if (string.IsNullOrWhiteSpace(updateConfig.AgenticAppId)) + { + logger.LogError("agenticAppId is not configured. Run 'a365 setup all' to complete setup."); + return; + } + // Configure GraphApiService with custom client app ID if available if (!string.IsNullOrWhiteSpace(updateConfig.ClientAppId)) { @@ -246,7 +261,8 @@ private static Command CreateMcpSubcommand( private static async Task ValidateDeploymentPrerequisitesAsync( string configPath, IConfigService configService, - IAzureValidator azureValidator, + IPrerequisiteRunner prerequisiteRunner, + AzureAuthValidator authValidator, CommandExecutor executor, ILogger logger) { @@ -255,7 +271,11 @@ private static Command CreateMcpSubcommand( if (configData == null) return null; // Validate Azure CLI authentication, subscription, and environment - if (!await azureValidator.ValidateAllAsync(configData.SubscriptionId)) + var checks = new List + { + new AzureAuthRequirementCheck(authValidator) + }; + if (!await prerequisiteRunner.RunAsync(checks, configData, logger)) { logger.LogError("Deployment cannot proceed without proper Azure CLI authentication and the correct subscription context"); return null; diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupCommand.cs index 08a913b1..8fd6c909 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupCommand.cs @@ -20,7 +20,9 @@ public static Command CreateCommand( CommandExecutor executor, DeploymentService deploymentService, IBotConfigurator botConfigurator, - IAzureValidator azureValidator, + IPrerequisiteRunner prerequisiteRunner, + AzureAuthValidator authValidator, + IAzureEnvironmentValidator environmentValidator, AzureWebAppCreator webAppCreator, PlatformDetector platformDetector, GraphApiService graphApiService, @@ -29,7 +31,7 @@ public static Command CreateCommand( FederatedCredentialService federatedCredentialService, IClientAppValidator clientAppValidator) { - var command = new Command("setup", + var command = new Command("setup", "Set up your Agent 365 environment with granular control over each step\n\n" + "Recommended execution order:\n" + " 0. a365 setup requirements # Check prerequisites (optional)\n" + @@ -46,16 +48,16 @@ public static Command CreateCommand( logger, configService, clientAppValidator)); command.AddCommand(InfrastructureSubcommand.CreateCommand( - logger, configService, azureValidator, webAppCreator, platformDetector, executor)); + logger, configService, prerequisiteRunner, authValidator, webAppCreator, platformDetector, executor)); command.AddCommand(BlueprintSubcommand.CreateCommand( - logger, configService, executor, azureValidator, webAppCreator, platformDetector, botConfigurator, graphApiService, blueprintService, clientAppValidator, blueprintLookupService, federatedCredentialService)); + logger, configService, executor, prerequisiteRunner, authValidator, webAppCreator, platformDetector, botConfigurator, graphApiService, blueprintService, clientAppValidator, blueprintLookupService, federatedCredentialService)); command.AddCommand(PermissionsSubcommand.CreateCommand( logger, configService, executor, graphApiService, blueprintService)); command.AddCommand(AllSubcommand.CreateCommand( - logger, configService, executor, botConfigurator, azureValidator, webAppCreator, platformDetector, graphApiService, blueprintService, clientAppValidator, blueprintLookupService, federatedCredentialService)); + logger, configService, executor, botConfigurator, prerequisiteRunner, authValidator, environmentValidator, webAppCreator, platformDetector, graphApiService, blueprintService, clientAppValidator, blueprintLookupService, federatedCredentialService)); return command; } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs index b67534c2..21cc8623 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs @@ -6,6 +6,7 @@ using Microsoft.Agents.A365.DevTools.Cli.Models; using Microsoft.Agents.A365.DevTools.Cli.Services; using Microsoft.Agents.A365.DevTools.Cli.Services.Internal; +using Microsoft.Agents.A365.DevTools.Cli.Services.Requirements.RequirementChecks; using Microsoft.Extensions.Logging; using System.CommandLine; @@ -26,7 +27,9 @@ public static Command CreateCommand( IConfigService configService, CommandExecutor executor, IBotConfigurator botConfigurator, - IAzureValidator azureValidator, + IPrerequisiteRunner prerequisiteRunner, + AzureAuthValidator authValidator, + IAzureEnvironmentValidator environmentValidator, AzureWebAppCreator webAppCreator, PlatformDetector platformDetector, GraphApiService graphApiService, @@ -206,57 +209,22 @@ public static Command CreateCommand( // PHASE 1: VALIDATE ALL PREREQUISITES UPFRONT logger.LogDebug("Validating all prerequisites..."); - var allErrors = new List(); - - // Validate Azure CLI authentication first - logger.LogDebug("Validating Azure CLI authentication..."); - if (!await azureValidator.ValidateAllAsync(setupConfig.SubscriptionId)) + var prereqChecks = new List { - allErrors.Add("Azure CLI authentication failed or subscription not set correctly"); - logger.LogError("Azure CLI authentication validation failed"); - } - else - { - logger.LogDebug("Azure CLI authentication: OK"); - } + new AzureAuthRequirementCheck(authValidator), + new ClientAppRequirementCheck(clientAppValidator) + }; - // Validate Infrastructure prerequisites if (!skipInfrastructure && setupConfig.NeedDeployment) - { - logger.LogDebug("Validating Infrastructure prerequisites..."); - var infraErrors = await InfrastructureSubcommand.ValidateAsync(setupConfig, azureValidator, CancellationToken.None); - if (infraErrors.Count > 0) - { - allErrors.AddRange(infraErrors.Select(e => $"Infrastructure: {e}")); - } - else - { - logger.LogDebug("Infrastructure prerequisites: OK"); - } - } + prereqChecks.Add(new InfrastructureRequirementCheck()); - // Validate Blueprint prerequisites - logger.LogDebug("Validating Blueprint prerequisites..."); - var blueprintErrors = await BlueprintSubcommand.ValidateAsync(setupConfig, azureValidator, clientAppValidator, CancellationToken.None); - if (blueprintErrors.Count > 0) - { - allErrors.AddRange(blueprintErrors.Select(e => $"Blueprint: {e}")); - } - else - { - logger.LogDebug("Blueprint prerequisites: OK"); - } + // Run advisory environment check (warning only, never blocks) + await environmentValidator.ValidateEnvironmentAsync(); - // Stop if any validation failed - if (allErrors.Count > 0) + if (!await prerequisiteRunner.RunAsync(prereqChecks, setupConfig, logger, CancellationToken.None)) { - logger.LogError("Setup cannot proceed due to validation failures:"); - foreach (var error in allErrors) - { - logger.LogError(" - {Error}", error); - } - logger.LogError("Please fix the errors above and try again"); - setupResults.Errors.AddRange(allErrors); + logger.LogError("Setup cannot proceed due to prerequisite validation failures. Please fix the errors above and try again."); + setupResults.Errors.Add("Prerequisite validation failed"); ExceptionHandler.ExitWithCleanup(1); } @@ -304,7 +272,8 @@ public static Command CreateCommand( setupConfig, config, executor, - azureValidator, + prerequisiteRunner, + authValidator, logger, skipInfrastructure, true, diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs index c1317519..5dbc1981 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs @@ -9,6 +9,7 @@ using Microsoft.Agents.A365.DevTools.Cli.Models; using Microsoft.Agents.A365.DevTools.Cli.Services; using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; +using Microsoft.Agents.A365.DevTools.Cli.Services.Requirements.RequirementChecks; using Microsoft.Agents.A365.DevTools.Cli.Services.Internal; using Microsoft.Extensions.Logging; using Microsoft.Graph; @@ -62,60 +63,12 @@ internal static class BlueprintSubcommand private const int ClientSecretValidationTimeoutSeconds = 10; private const string MicrosoftLoginOAuthTokenEndpoint = "https://login.microsoftonline.com/{0}/oauth2/v2.0/token"; - /// - /// Validates blueprint prerequisites without performing any actions. - /// - public static async Task> ValidateAsync( - Models.Agent365Config config, - IAzureValidator azureValidator, - IClientAppValidator clientAppValidator, - CancellationToken cancellationToken = default) - { - var errors = new List(); - - if (string.IsNullOrWhiteSpace(config.ClientAppId)) - { - errors.Add("clientAppId is required in configuration"); - errors.Add("Please configure a custom client app in your tenant with required permissions"); - errors.Add($"See {ConfigConstants.Agent365CliDocumentationUrl} for setup instructions"); - return errors; - } - - // Validate client app exists and has required permissions - try - { - await clientAppValidator.EnsureValidClientAppAsync( - config.ClientAppId, - config.TenantId, - cancellationToken); - } - catch (ClientAppValidationException ex) - { - // Add issue description and error details - errors.Add(ex.IssueDescription); - errors.AddRange(ex.ErrorDetails); - - // Add mitigation steps if available - if (ex.MitigationSteps.Count > 0) - { - errors.AddRange(ex.MitigationSteps); - } - } - catch (Exception ex) - { - // Catch any unexpected validation errors (Graph API failures, etc.) - errors.Add($"Client app validation failed: {ex.Message}"); - errors.Add("Ensure Azure CLI is authenticated and you have access to the tenant."); - } - - return errors; - } - public static Command CreateCommand( ILogger logger, IConfigService configService, CommandExecutor executor, - IAzureValidator azureValidator, + IPrerequisiteRunner prerequisiteRunner, + AzureAuthValidator authValidator, AzureWebAppCreator webAppCreator, PlatformDetector platformDetector, IBotConfigurator botConfigurator, @@ -303,7 +256,8 @@ await CreateBlueprintImplementationAsync( setupConfig, config, executor, - azureValidator, + prerequisiteRunner, + authValidator, logger, false, false, @@ -367,7 +321,8 @@ public static async Task CreateBlueprintImplementationA Models.Agent365Config setupConfig, FileInfo config, CommandExecutor executor, - IAzureValidator azureValidator, + IPrerequisiteRunner prerequisiteRunner, + AzureAuthValidator authValidator, ILogger logger, bool skipInfrastructure, bool isSetupAll, @@ -401,7 +356,11 @@ public static async Task CreateBlueprintImplementationA logger.LogInformation("==> Creating Agent Blueprint"); // Validate Azure authentication - if (!await azureValidator.ValidateAllAsync(setupConfig.SubscriptionId)) + var authChecks = new List + { + new AzureAuthRequirementCheck(authValidator) + }; + if (!await prerequisiteRunner.RunAsync(authChecks, setupConfig, logger, cancellationToken)) { return new BlueprintCreationResult { diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/InfrastructureSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/InfrastructureSubcommand.cs index dfe369c2..3856071e 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/InfrastructureSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/InfrastructureSubcommand.cs @@ -6,6 +6,7 @@ using Microsoft.Agents.A365.DevTools.Cli.Models; using Microsoft.Agents.A365.DevTools.Cli.Services; using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; +using Microsoft.Agents.A365.DevTools.Cli.Services.Requirements.RequirementChecks; using Microsoft.Extensions.Logging; using System.CommandLine; using System.Text.Json; @@ -25,83 +26,11 @@ public static class InfrastructureSubcommand private const int InitialRetryDelayMs = 500; private const int MaxRetryDelayMs = 5000; // Cap exponential backoff at 5 seconds - /// - /// Validates infrastructure prerequisites without performing any actions. - /// Includes validation of App Service Plan SKU and provides recommendations. - /// - public static Task> ValidateAsync( - Agent365Config config, - IAzureValidator azureValidator, - CancellationToken cancellationToken = default) - { - var errors = new List(); - - if (!config.NeedDeployment) - { - return Task.FromResult(errors); - } - - if (string.IsNullOrWhiteSpace(config.SubscriptionId)) - errors.Add("subscriptionId is required for Azure hosting"); - - if (string.IsNullOrWhiteSpace(config.ResourceGroup)) - errors.Add("resourceGroup is required for Azure hosting"); - - if (string.IsNullOrWhiteSpace(config.AppServicePlanName)) - errors.Add("appServicePlanName is required for Azure hosting"); - - if (string.IsNullOrWhiteSpace(config.WebAppName)) - errors.Add("webAppName is required for Azure hosting"); - - if (string.IsNullOrWhiteSpace(config.Location)) - errors.Add("location is required for Azure hosting"); - - // Validate App Service Plan SKU - var sku = string.IsNullOrWhiteSpace(config.AppServicePlanSku) - ? ConfigConstants.DefaultAppServicePlanSku - : config.AppServicePlanSku; - - if (!IsValidAppServicePlanSku(sku)) - { - errors.Add($"Invalid appServicePlanSku '{sku}'. Valid SKUs: F1 (Free), B1/B2/B3 (Basic), S1/S2/S3 (Standard), P1V2/P2V2/P3V2 (Premium V2), P1V3/P2V3/P3V3 (Premium V3)"); - } - // Note: B1 quota warning is now logged at execution time with actual quota check - - return Task.FromResult(errors); - } - - /// - /// Validates if the provided SKU is a valid App Service Plan SKU. - /// - private static bool IsValidAppServicePlanSku(string sku) - { - if (string.IsNullOrWhiteSpace(sku)) - return false; - - // Common valid SKUs (case-insensitive) - var validSkus = new[] - { - // Free tier - "F1", - // Basic tier - "B1", "B2", "B3", - // Standard tier - "S1", "S2", "S3", - // Premium V2 - "P1V2", "P2V2", "P3V2", - // Premium V3 - "P1V3", "P2V3", "P3V3", - // Isolated (less common) - "I1", "I2", "I3", - "I1V2", "I2V2", "I3V2" - }; - - return validSkus.Contains(sku, StringComparer.OrdinalIgnoreCase); - } public static Command CreateCommand( ILogger logger, IConfigService configService, - IAzureValidator azureValidator, + IPrerequisiteRunner prerequisiteRunner, + AzureAuthValidator authValidator, AzureWebAppCreator webAppCreator, PlatformDetector platformDetector, CommandExecutor executor) @@ -158,8 +87,12 @@ public static Command CreateCommand( var setupConfig = await configService.LoadAsync(config.FullName); if (setupConfig.NeedDeployment) { - // Validate Azure CLI authentication, subscription, and environment - if (!await azureValidator.ValidateAllAsync(setupConfig.SubscriptionId)) + var checks = new List + { + new AzureAuthRequirementCheck(authValidator), + new InfrastructureRequirementCheck() + }; + if (!await prerequisiteRunner.RunAsync(checks, setupConfig, logger, CancellationToken.None)) { ExceptionHandler.ExitWithCleanup(1); } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs index fa64b85b..a8972712 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs @@ -90,7 +90,9 @@ static async Task Main(string[] args) var configService = serviceProvider.GetRequiredService(); var executor = serviceProvider.GetRequiredService(); var authService = serviceProvider.GetRequiredService(); - var azureValidator = serviceProvider.GetRequiredService(); + var prerequisiteRunner = serviceProvider.GetRequiredService(); + var authValidator = serviceProvider.GetRequiredService(); + var environmentValidator = serviceProvider.GetRequiredService(); var toolingService = serviceProvider.GetRequiredService(); // Get services needed by commands @@ -111,11 +113,11 @@ static async Task Main(string[] args) rootCommand.AddCommand(DevelopCommand.CreateCommand(developLogger, configService, executor, authService, graphApiService, agentBlueprintService, processService)); rootCommand.AddCommand(DevelopMcpCommand.CreateCommand(developLogger, toolingService)); rootCommand.AddCommand(SetupCommand.CreateCommand(setupLogger, configService, executor, - deploymentService, botConfigurator, azureValidator, webAppCreator, platformDetector, graphApiService, agentBlueprintService, blueprintLookupService, federatedCredentialService, clientAppValidator)); + deploymentService, botConfigurator, prerequisiteRunner, authValidator, environmentValidator, webAppCreator, platformDetector, graphApiService, agentBlueprintService, blueprintLookupService, federatedCredentialService, clientAppValidator)); rootCommand.AddCommand(CreateInstanceCommand.CreateCommand(createInstanceLogger, configService, executor, - botConfigurator, graphApiService, azureValidator)); + botConfigurator, graphApiService)); rootCommand.AddCommand(DeployCommand.CreateCommand(deployLogger, configService, executor, - deploymentService, azureValidator, graphApiService, agentBlueprintService)); + deploymentService, prerequisiteRunner, authValidator, graphApiService, agentBlueprintService)); // Register ConfigCommand var configLoggerFactory = serviceProvider.GetRequiredService(); @@ -125,7 +127,7 @@ static async Task Main(string[] args) var confirmationProvider = serviceProvider.GetRequiredService(); rootCommand.AddCommand(ConfigCommand.CreateCommand(configLogger, wizardService: wizardService, clientAppValidator: clientAppValidator)); rootCommand.AddCommand(QueryEntraCommand.CreateCommand(queryEntraLogger, configService, executor, graphApiService, agentBlueprintService)); - rootCommand.AddCommand(CleanupCommand.CreateCommand(cleanupLogger, configService, botConfigurator, executor, agentBlueprintService, confirmationProvider, federatedCredentialService)); + rootCommand.AddCommand(CleanupCommand.CreateCommand(cleanupLogger, configService, botConfigurator, executor, agentBlueprintService, confirmationProvider, federatedCredentialService, prerequisiteRunner, authValidator)); rootCommand.AddCommand(PublishCommand.CreateCommand(publishLogger, configService, agentPublishService, graphApiService, agentBlueprintService, manifestTemplateService)); // Wrap all command handlers with exception handling @@ -228,12 +230,12 @@ private static void ConfigureServices(IServiceCollection services, LogLevel mini return new Agent365ToolingService(configService, authService, logger, environment); }); - // Add Azure validators (individual validators for composition) + // Add Azure validators services.AddSingleton(); services.AddSingleton(); - // Add unified Azure validator - services.AddSingleton(); + // Add prerequisite runner + services.AddSingleton(); // Add multi-platform deployment services services.AddSingleton(); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AzureAuthValidator.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AzureAuthValidator.cs index c81b6094..49e28eb1 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AzureAuthValidator.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AzureAuthValidator.cs @@ -26,7 +26,7 @@ public AzureAuthValidator(ILogger logger, CommandExecutor ex /// /// The expected subscription ID to validate against. If null, only checks authentication. /// True if authenticated and subscription matches (if specified), false otherwise. - public async Task ValidateAuthenticationAsync(string? expectedSubscriptionId = null) + public virtual async Task ValidateAuthenticationAsync(string? expectedSubscriptionId = null) { try { diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AzureValidator.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AzureValidator.cs deleted file mode 100644 index eb6a0b54..00000000 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AzureValidator.cs +++ /dev/null @@ -1,54 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -using Microsoft.Extensions.Logging; - -namespace Microsoft.Agents.A365.DevTools.Cli.Services; - -/// -/// Unified Azure validator that orchestrates all Azure-related validations. -/// -public interface IAzureValidator -{ - /// - /// Validates Azure CLI authentication, subscription, and environment. - /// - /// Expected subscription ID - /// True if all validations pass - Task ValidateAllAsync(string subscriptionId); -} - -public class AzureValidator : IAzureValidator -{ - private readonly AzureAuthValidator _authValidator; - private readonly IAzureEnvironmentValidator _environmentValidator; - private readonly ILogger _logger; - - public AzureValidator( - AzureAuthValidator authValidator, - IAzureEnvironmentValidator environmentValidator, - ILogger logger) - { - _authValidator = authValidator; - _environmentValidator = environmentValidator; - _logger = logger; - } - - /// - public async Task ValidateAllAsync(string subscriptionId) - { - _logger.LogDebug("Validating Azure CLI authentication and subscription..."); - - // Authentication validation (critical - stops execution if failed) - if (!await _authValidator.ValidateAuthenticationAsync(subscriptionId)) - { - _logger.LogError("Setup cannot proceed without proper Azure CLI authentication and subscription"); - return false; - } - - // Environment validation (warnings only - doesn't stop execution) - await _environmentValidator.ValidateEnvironmentAsync(); - - return true; - } -} \ No newline at end of file diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/IPrerequisiteRunner.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/IPrerequisiteRunner.cs new file mode 100644 index 00000000..39532480 --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/IPrerequisiteRunner.cs @@ -0,0 +1,30 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Microsoft.Agents.A365.DevTools.Cli.Models; +using Microsoft.Agents.A365.DevTools.Cli.Services.Requirements; +using Microsoft.Extensions.Logging; + +namespace Microsoft.Agents.A365.DevTools.Cli.Services; + +/// +/// Runs a set of prerequisite checks before a command executes. +/// Returns false and logs actionable errors if any blocking check fails. +/// Warnings are logged but do not block execution. +/// +public interface IPrerequisiteRunner +{ + /// + /// Runs all checks in order. + /// + /// The prerequisite checks to run. + /// The current Agent 365 configuration. + /// Logger for output. + /// Cancellation token. + /// True if all blocking checks pass; false if any check fails. + Task RunAsync( + IEnumerable checks, + Agent365Config config, + ILogger logger, + CancellationToken cancellationToken = default); +} diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/ISubCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/ISubCommand.cs deleted file mode 100644 index bcf4298d..00000000 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/ISubCommand.cs +++ /dev/null @@ -1,22 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -using Microsoft.Agents.A365.DevTools.Cli.Models; - -namespace Microsoft.Agents.A365.DevTools.Cli.Services; - -/// -/// Interface for subcommands that require validation before execution. -/// Implements separation of validation and execution phases to fail fast on configuration issues. -/// -public interface ISubCommand -{ - /// - /// Validates prerequisites for the subcommand without performing any actions. - /// This should check configuration, authentication, and environment requirements. - /// - /// The Agent365 configuration - /// Cancellation token - /// List of validation errors, empty if validation passes - Task> ValidateAsync(Agent365Config config, CancellationToken cancellationToken = default); -} diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/PrerequisiteRunner.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/PrerequisiteRunner.cs new file mode 100644 index 00000000..c13506d3 --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/PrerequisiteRunner.cs @@ -0,0 +1,46 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Microsoft.Agents.A365.DevTools.Cli.Models; +using Microsoft.Agents.A365.DevTools.Cli.Services.Requirements; +using Microsoft.Extensions.Logging; + +namespace Microsoft.Agents.A365.DevTools.Cli.Services; + +/// +/// Runs prerequisite checks for a command and logs failures with actionable guidance. +/// +public class PrerequisiteRunner : IPrerequisiteRunner +{ + /// + public async Task RunAsync( + IEnumerable checks, + Agent365Config config, + ILogger logger, + CancellationToken cancellationToken = default) + { + var passed = true; + + foreach (var check in checks) + { + var result = await check.CheckAsync(config, logger, cancellationToken); + + if (!result.Passed) + { + passed = false; + + if (!string.IsNullOrWhiteSpace(result.ErrorMessage)) + logger.LogError("{CheckName}: {ErrorMessage}", check.Name, result.ErrorMessage); + + if (!string.IsNullOrWhiteSpace(result.ResolutionGuidance)) + logger.LogError(" Resolution: {ResolutionGuidance}", result.ResolutionGuidance); + } + else if (result.IsWarning && !string.IsNullOrWhiteSpace(result.Details)) + { + logger.LogWarning("{CheckName}: {Details}", check.Name, result.Details); + } + } + + return passed; + } +} diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/AzureAuthRequirementCheck.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/AzureAuthRequirementCheck.cs new file mode 100644 index 00000000..3fe62dc2 --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/AzureAuthRequirementCheck.cs @@ -0,0 +1,50 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Microsoft.Agents.A365.DevTools.Cli.Models; +using Microsoft.Extensions.Logging; + +namespace Microsoft.Agents.A365.DevTools.Cli.Services.Requirements.RequirementChecks; + +/// +/// Validates Azure CLI authentication and active subscription. +/// Delegates entirely to AzureAuthValidator which handles all user-facing logging. +/// +public class AzureAuthRequirementCheck : RequirementCheck +{ + private readonly AzureAuthValidator _authValidator; + + public AzureAuthRequirementCheck(AzureAuthValidator authValidator) + { + _authValidator = authValidator ?? throw new ArgumentNullException(nameof(authValidator)); + } + + /// + public override string Name => "Azure Authentication"; + + /// + public override string Description => "Validates Azure CLI authentication and active subscription"; + + /// + public override string Category => "Azure"; + + /// + public override async Task CheckAsync( + Agent365Config config, + ILogger logger, + CancellationToken cancellationToken = default) + { + // AzureAuthValidator logs all detailed user-facing messages internally. + // This adapter converts the bool result into a RequirementCheckResult. + var authenticated = await _authValidator.ValidateAuthenticationAsync(config.SubscriptionId); + + if (!authenticated) + { + return RequirementCheckResult.Failure( + "Azure CLI authentication failed or the active subscription does not match the configured subscriptionId", + "Run 'az login' to authenticate, then 'az account set --subscription ' to select the correct subscription"); + } + + return RequirementCheckResult.Success(); + } +} diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/InfrastructureRequirementCheck.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/InfrastructureRequirementCheck.cs new file mode 100644 index 00000000..4c690eb1 --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/InfrastructureRequirementCheck.cs @@ -0,0 +1,93 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Microsoft.Agents.A365.DevTools.Cli.Constants; +using Microsoft.Agents.A365.DevTools.Cli.Models; +using Microsoft.Extensions.Logging; + +namespace Microsoft.Agents.A365.DevTools.Cli.Services.Requirements.RequirementChecks; + +/// +/// Validates Azure infrastructure configuration fields required for Azure-hosted deployments. +/// Skips all checks when NeedDeployment is false (external messaging endpoint scenario). +/// No external service calls — pure configuration validation. +/// +public class InfrastructureRequirementCheck : RequirementCheck +{ + /// + public override string Name => "Infrastructure Configuration"; + + /// + public override string Description => "Validates Azure infrastructure configuration fields (subscription, resource group, app service plan, web app, location, SKU)"; + + /// + public override string Category => "Configuration"; + + /// + public override Task CheckAsync( + Agent365Config config, + ILogger logger, + CancellationToken cancellationToken = default) + { + if (!config.NeedDeployment) + return Task.FromResult(RequirementCheckResult.Success()); + + var errors = new List(); + + if (string.IsNullOrWhiteSpace(config.SubscriptionId)) + errors.Add("subscriptionId is required for Azure hosting"); + + if (string.IsNullOrWhiteSpace(config.ResourceGroup)) + errors.Add("resourceGroup is required for Azure hosting"); + + if (string.IsNullOrWhiteSpace(config.AppServicePlanName)) + errors.Add("appServicePlanName is required for Azure hosting"); + + if (string.IsNullOrWhiteSpace(config.WebAppName)) + errors.Add("webAppName is required for Azure hosting"); + + if (string.IsNullOrWhiteSpace(config.Location)) + errors.Add("location is required for Azure hosting"); + + var sku = string.IsNullOrWhiteSpace(config.AppServicePlanSku) + ? ConfigConstants.DefaultAppServicePlanSku + : config.AppServicePlanSku; + + if (!IsValidAppServicePlanSku(sku)) + errors.Add($"Invalid appServicePlanSku '{sku}'. Valid SKUs: F1 (Free), B1/B2/B3 (Basic), S1/S2/S3 (Standard), P1V2/P2V2/P3V2 (Premium V2), P1V3/P2V3/P3V3 (Premium V3)"); + + if (errors.Count > 0) + { + return Task.FromResult(RequirementCheckResult.Failure( + string.Join("; ", errors), + "Update the missing or invalid fields in a365.config.json and run again")); + } + + return Task.FromResult(RequirementCheckResult.Success()); + } + + private static bool IsValidAppServicePlanSku(string sku) + { + if (string.IsNullOrWhiteSpace(sku)) + return false; + + var validSkus = new[] + { + // Free tier + "F1", + // Basic tier + "B1", "B2", "B3", + // Standard tier + "S1", "S2", "S3", + // Premium V2 + "P1V2", "P2V2", "P3V2", + // Premium V3 + "P1V3", "P2V3", "P3V3", + // Isolated (less common) + "I1", "I2", "I3", + "I1V2", "I2V2", "I3V2" + }; + + return validSkus.Contains(sku, StringComparer.OrdinalIgnoreCase); + } +} diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BlueprintSubcommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BlueprintSubcommandTests.cs index 048f45f2..ad36bc92 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BlueprintSubcommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BlueprintSubcommandTests.cs @@ -6,7 +6,9 @@ using Microsoft.Agents.A365.DevTools.Cli.Models; using Microsoft.Agents.A365.DevTools.Cli.Services; using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; +using Microsoft.Agents.A365.DevTools.Cli.Services.Requirements; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using NSubstitute; using System.CommandLine; using System.CommandLine.Builder; @@ -26,7 +28,8 @@ public class BlueprintSubcommandTests private readonly ILogger _mockLogger; private readonly IConfigService _mockConfigService; private readonly CommandExecutor _mockExecutor; - private readonly IAzureValidator _mockAzureValidator; + private readonly IPrerequisiteRunner _mockPrerequisiteRunner; + private readonly AzureAuthValidator _mockAuthValidator; private readonly AzureWebAppCreator _mockWebAppCreator; private readonly PlatformDetector _mockPlatformDetector; private readonly IBotConfigurator _mockBotConfigurator; @@ -42,7 +45,8 @@ public BlueprintSubcommandTests() _mockConfigService = Substitute.For(); var mockExecutorLogger = Substitute.For>(); _mockExecutor = Substitute.ForPartsOf(mockExecutorLogger); - _mockAzureValidator = Substitute.For(); + _mockPrerequisiteRunner = Substitute.For(); + _mockAuthValidator = Substitute.ForPartsOf(NullLogger.Instance, _mockExecutor); _mockWebAppCreator = Substitute.ForPartsOf(Substitute.For>()); var mockPlatformDetectorLogger = Substitute.For>(); _mockPlatformDetector = Substitute.ForPartsOf(mockPlatformDetectorLogger); @@ -62,7 +66,8 @@ public void CreateCommand_ShouldHaveCorrectName() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockPrerequisiteRunner, + _mockAuthValidator, _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, @@ -80,7 +85,8 @@ public void CreateCommand_ShouldHaveDescription() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockPrerequisiteRunner, + _mockAuthValidator, _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, @@ -99,7 +105,8 @@ public void CreateCommand_ShouldHaveConfigOption() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockPrerequisiteRunner, + _mockAuthValidator, _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, @@ -120,7 +127,8 @@ public void CreateCommand_ShouldHaveVerboseOption() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockPrerequisiteRunner, + _mockAuthValidator, _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, @@ -141,7 +149,8 @@ public void CreateCommand_ShouldHaveDryRunOption() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockPrerequisiteRunner, + _mockAuthValidator, _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, @@ -161,7 +170,8 @@ public void CreateCommand_ShouldHaveSkipRequirementsOption() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockPrerequisiteRunner, + _mockAuthValidator, _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, @@ -190,7 +200,8 @@ public async Task DryRun_ShouldLoadConfigAndNotExecute() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockPrerequisiteRunner, + _mockAuthValidator, _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, @@ -205,7 +216,6 @@ public async Task DryRun_ShouldLoadConfigAndNotExecute() // Assert result.Should().Be(0); await _mockConfigService.Received(1).LoadAsync(Arg.Any(), Arg.Any()); - await _mockAzureValidator.DidNotReceiveWithAnyArgs().ValidateAllAsync(default!); } [Fact] @@ -225,7 +235,8 @@ public async Task DryRun_ShouldDisplayBlueprintInformation() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockPrerequisiteRunner, + _mockAuthValidator, _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, @@ -262,20 +273,25 @@ public async Task CreateBlueprintImplementation_WithMissingDisplayName_ShouldThr var configFile = new FileInfo("test-config.json"); - _mockAzureValidator.ValidateAllAsync(Arg.Any()) + _mockPrerequisiteRunner.RunAsync( + Arg.Any>(), + Arg.Any(), + Arg.Any(), + Arg.Any()) .Returns(true); // Note: Since DelegatedConsentService needs to run and will fail with invalid tenant, // the method returns false rather than throwing for missing display name upfront. // The display name check happens after consent, so this test verifies // the method can handle failures gracefully. - + // Act var result = await BlueprintSubcommand.CreateBlueprintImplementationAsync( config, configFile, _mockExecutor, - _mockAzureValidator, + _mockPrerequisiteRunner, + _mockAuthValidator, _mockLogger, skipInfrastructure: false, isSetupAll: false, @@ -305,15 +321,20 @@ public async Task CreateBlueprintImplementation_WithAzureValidationFailure_Shoul var configFile = new FileInfo("test-config.json"); - _mockAzureValidator.ValidateAllAsync(Arg.Any()) - .Returns(false); // Validation fails + _mockPrerequisiteRunner.RunAsync( + Arg.Any>(), + Arg.Any(), + Arg.Any(), + Arg.Any()) + .Returns(false); // Auth check fails // Act var result = await BlueprintSubcommand.CreateBlueprintImplementationAsync( config, configFile, _mockExecutor, - _mockAzureValidator, + _mockPrerequisiteRunner, + _mockAuthValidator, _mockLogger, skipInfrastructure: false, isSetupAll: false, @@ -326,7 +347,11 @@ public async Task CreateBlueprintImplementation_WithAzureValidationFailure_Shoul result.Should().NotBeNull(); result.BlueprintCreated.Should().BeFalse(); result.EndpointRegistered.Should().BeFalse(); - await _mockAzureValidator.Received(1).ValidateAllAsync(config.SubscriptionId); + await _mockPrerequisiteRunner.Received(1).RunAsync( + Arg.Any>(), + Arg.Any(), + Arg.Any(), + Arg.Any()); } [Fact] @@ -337,7 +362,8 @@ public void CommandDescription_ShouldMentionRequiredPermissions() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockPrerequisiteRunner, + _mockAuthValidator, _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, @@ -365,7 +391,8 @@ public async Task DryRun_WithCustomConfigPath_ShouldLoadCorrectFile() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockPrerequisiteRunner, + _mockAuthValidator, _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, @@ -401,7 +428,8 @@ public async Task DryRun_ShouldNotCreateServicePrincipal() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockPrerequisiteRunner, + _mockAuthValidator, _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, @@ -429,7 +457,8 @@ public void CreateCommand_ShouldHandleAllOptions() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockPrerequisiteRunner, + _mockAuthValidator, _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, @@ -455,7 +484,8 @@ public async Task DryRun_WithMissingConfig_ShouldHandleGracefully() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockPrerequisiteRunner, + _mockAuthValidator, _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, @@ -477,7 +507,8 @@ public void CreateCommand_DefaultConfigPath_ShouldBeA365ConfigJson() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockPrerequisiteRunner, + _mockAuthValidator, _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, @@ -504,7 +535,11 @@ public async Task CreateBlueprintImplementation_ShouldLogProgressMessages() var configFile = new FileInfo("test-config.json"); - _mockAzureValidator.ValidateAllAsync(Arg.Any()) + _mockPrerequisiteRunner.RunAsync( + Arg.Any>(), + Arg.Any(), + Arg.Any(), + Arg.Any()) .Returns(false); // Fail fast for this test // Act @@ -512,7 +547,8 @@ public async Task CreateBlueprintImplementation_ShouldLogProgressMessages() config, configFile, _mockExecutor, - _mockAzureValidator, + _mockPrerequisiteRunner, + _mockAuthValidator, _mockLogger, skipInfrastructure: false, isSetupAll: false, @@ -543,7 +579,8 @@ public void CommandDescription_ShouldBeInformativeAndActionable() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockPrerequisiteRunner, + _mockAuthValidator, _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, @@ -571,7 +608,8 @@ public async Task DryRun_WithVerboseFlag_ShouldSucceed() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockPrerequisiteRunner, + _mockAuthValidator, _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, @@ -605,7 +643,8 @@ public async Task DryRun_ShouldShowWhatWouldBeDone() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockPrerequisiteRunner, + _mockAuthValidator, _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, @@ -637,7 +676,8 @@ public void CreateCommand_ShouldBeUsableInCommandPipeline() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockPrerequisiteRunner, + _mockAuthValidator, _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, @@ -1327,7 +1367,8 @@ public void CreateCommand_ShouldHaveUpdateEndpointOption() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockPrerequisiteRunner, + _mockAuthValidator, _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, @@ -1661,7 +1702,8 @@ public async Task SetHandler_WithClientAppId_ShouldConfigureGraphApiService() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockPrerequisiteRunner, + _mockAuthValidator, _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, @@ -1696,7 +1738,8 @@ public async Task SetHandler_WithoutClientAppId_ShouldNotConfigureGraphApiServic _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockPrerequisiteRunner, + _mockAuthValidator, _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, @@ -1731,7 +1774,8 @@ public async Task SetHandler_WithWhitespaceClientAppId_ShouldNotConfigureGraphAp _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockPrerequisiteRunner, + _mockAuthValidator, _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandBotEndpointTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandBotEndpointTests.cs index 52ca8f69..4a3f72ed 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandBotEndpointTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandBotEndpointTests.cs @@ -2,9 +2,11 @@ // Licensed under the MIT License. using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Agents.A365.DevTools.Cli.Commands; using Microsoft.Agents.A365.DevTools.Cli.Models; using Microsoft.Agents.A365.DevTools.Cli.Services; +using Microsoft.Agents.A365.DevTools.Cli.Services.Requirements; using NSubstitute; using Xunit; @@ -21,6 +23,8 @@ public class CleanupCommandBotEndpointTests private readonly FederatedCredentialService _federatedCredentialService; private readonly IMicrosoftGraphTokenProvider _mockTokenProvider; private readonly IConfirmationProvider _mockConfirmationProvider; + private readonly IPrerequisiteRunner _mockPrerequisiteRunner; + private readonly AzureAuthValidator _mockAuthValidator; public CleanupCommandBotEndpointTests() { @@ -76,6 +80,15 @@ public CleanupCommandBotEndpointTests() _mockConfirmationProvider = Substitute.For(); _mockConfirmationProvider.ConfirmAsync(Arg.Any()).Returns(true); _mockConfirmationProvider.ConfirmWithTypedResponseAsync(Arg.Any(), Arg.Any()).Returns(true); + + _mockPrerequisiteRunner = Substitute.For(); + _mockPrerequisiteRunner.RunAsync( + Arg.Any>(), + Arg.Any(), + Arg.Any(), + Arg.Any()) + .Returns(true); + _mockAuthValidator = Substitute.ForPartsOf(NullLogger.Instance, _mockExecutor); } [Fact] @@ -102,13 +115,15 @@ public void BotConfigurator_DeleteEndpoint_ShouldBeCalledIndependentlyOfWebApp() AgentBlueprintId = "blueprint-id" }; var command = CleanupCommand.CreateCommand( - _mockLogger, - _mockConfigService, - _mockBotConfigurator, - _mockExecutor, + _mockLogger, + _mockConfigService, + _mockBotConfigurator, + _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, - _federatedCredentialService); + _federatedCredentialService, + _mockPrerequisiteRunner, + _mockAuthValidator); Assert.NotNull(command); Assert.Equal("cleanup", command.Name); diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandTests.cs index cb0d0d2b..e40ec403 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandTests.cs @@ -4,9 +4,11 @@ using System.CommandLine; using FluentAssertions; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Agents.A365.DevTools.Cli.Commands; using Microsoft.Agents.A365.DevTools.Cli.Models; using Microsoft.Agents.A365.DevTools.Cli.Services; +using Microsoft.Agents.A365.DevTools.Cli.Services.Requirements; using NSubstitute; using Xunit; @@ -24,6 +26,8 @@ public class CleanupCommandTests private readonly FederatedCredentialService _federatedCredentialService; private readonly IMicrosoftGraphTokenProvider _mockTokenProvider; private readonly IConfirmationProvider _mockConfirmationProvider; + private readonly IPrerequisiteRunner _mockPrerequisiteRunner; + private readonly AzureAuthValidator _mockAuthValidator; public CleanupCommandTests() { @@ -66,6 +70,14 @@ public CleanupCommandTests() _mockConfirmationProvider = Substitute.For(); _mockConfirmationProvider.ConfirmAsync(Arg.Any()).Returns(true); _mockConfirmationProvider.ConfirmWithTypedResponseAsync(Arg.Any(), Arg.Any()).Returns(true); + _mockPrerequisiteRunner = Substitute.For(); + _mockPrerequisiteRunner.RunAsync( + Arg.Any>(), + Arg.Any(), + Arg.Any(), + Arg.Any()) + .Returns(true); + _mockAuthValidator = Substitute.ForPartsOf(NullLogger.Instance, _mockExecutor); } [Fact(Skip = "Test requires interactive confirmation - cleanup commands now enforce user confirmation instead of --force")] @@ -75,7 +87,7 @@ public async Task CleanupAzure_WithValidConfig_ShouldExecuteResourceDeleteComman var config = CreateValidConfig(); _mockConfigService.LoadAsync(Arg.Any(), Arg.Any()).Returns(config); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService, _mockPrerequisiteRunner, _mockAuthValidator); var args = new[] { "cleanup", "azure", "--config", "test.json" }; // Act @@ -104,7 +116,7 @@ public async Task CleanupInstance_WithValidConfig_ShouldReturnSuccess() _mockConfigService.LoadAsync(Arg.Any(), Arg.Any()).Returns(config); _mockBotConfigurator.DeleteEndpointWithAgentBlueprintAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) .Returns(Task.FromResult(true)); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService, _mockPrerequisiteRunner, _mockAuthValidator); var args = new[] { "cleanup", "instance", "--config", "test.json" }; var originalIn = Console.In; @@ -135,7 +147,7 @@ public async Task Cleanup_WithoutSubcommand_ShouldExecuteCompleteCleanup() var config = CreateValidConfig(); _mockConfigService.LoadAsync(Arg.Any(), Arg.Any()).Returns(config); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService, _mockPrerequisiteRunner, _mockAuthValidator); var args = new[] { "cleanup", "--config", "test.json" }; // Act @@ -165,7 +177,7 @@ public async Task CleanupAzure_WithMissingWebAppName_ShouldStillExecuteCommand() var config = CreateConfigWithMissingWebApp(); // Create config without web app name _mockConfigService.LoadAsync(Arg.Any(), Arg.Any()).Returns(config); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService, _mockPrerequisiteRunner, _mockAuthValidator); var args = new[] { "cleanup", "azure", "--config", "test.json" }; // Act @@ -192,7 +204,7 @@ public async Task CleanupCommand_WithInvalidConfigFile_ShouldReturnError() _mockBotConfigurator.DeleteEndpointWithAgentBlueprintAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) .Returns(Task.FromResult(false)); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService, _mockPrerequisiteRunner, _mockAuthValidator); var args = new[] { "cleanup", "azure", "--config", "invalid.json" }; // Act @@ -212,7 +224,7 @@ await _mockExecutor.DidNotReceive().ExecuteAsync( public void CleanupCommand_ShouldHaveCorrectSubcommands() { // Arrange & Act - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService, _mockPrerequisiteRunner, _mockAuthValidator); // Assert - Verify command structure (what users see) Assert.Equal("cleanup", command.Name); @@ -231,7 +243,7 @@ public void CleanupCommand_ShouldHaveCorrectSubcommands() public void CleanupCommand_ShouldHaveDefaultHandlerOptions() { // Arrange & Act - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService, _mockPrerequisiteRunner, _mockAuthValidator); // Assert - Verify parent command has options for default handler var optionNames = command.Options.Select(opt => opt.Name).ToList(); @@ -244,7 +256,7 @@ public void CleanupCommand_ShouldHaveDefaultHandlerOptions() public void CleanupSubcommands_ShouldHaveRequiredOptions() { // Arrange & Act - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService, _mockPrerequisiteRunner, _mockAuthValidator); var blueprintCommand = command.Subcommands.First(sc => sc.Name == "blueprint"); // Assert - Verify user-facing options @@ -266,7 +278,7 @@ public async Task CleanupBlueprint_WithValidConfig_ShouldReturnSuccess() _mockConfirmationProvider.ConfirmAsync(Arg.Any()).Returns(true); var stubbedBlueprintService = CreateStubbedBlueprintService(); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, stubbedBlueprintService, _mockConfirmationProvider, _federatedCredentialService); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, stubbedBlueprintService, _mockConfirmationProvider, _federatedCredentialService, _mockPrerequisiteRunner, _mockAuthValidator); var args = new[] { "cleanup", "blueprint", "--config", "test.json" }; // Act @@ -326,7 +338,7 @@ public async Task CleanupBlueprint_WithInstances_DeletesInstancesBeforeBlueprint var command = CleanupCommand.CreateCommand( _mockLogger, _mockConfigService, _mockBotConfigurator, - _mockExecutor, spyService, _mockConfirmationProvider, _federatedCredentialService); + _mockExecutor, spyService, _mockConfirmationProvider, _federatedCredentialService, _mockPrerequisiteRunner, _mockAuthValidator); var args = new[] { "cleanup", "blueprint", "--config", "test.json" }; // Act @@ -367,7 +379,7 @@ public async Task CleanupBlueprint_WithNoInstances_ProceedsAsNormal() var command = CleanupCommand.CreateCommand( _mockLogger, _mockConfigService, _mockBotConfigurator, - _mockExecutor, spyService, _mockConfirmationProvider, _federatedCredentialService); + _mockExecutor, spyService, _mockConfirmationProvider, _federatedCredentialService, _mockPrerequisiteRunner, _mockAuthValidator); var args = new[] { "cleanup", "blueprint", "--config", "test.json" }; // Act @@ -415,7 +427,7 @@ public async Task CleanupBlueprint_InstanceDeletionFails_WarnsAndContinuesToBlue var command = CleanupCommand.CreateCommand( _mockLogger, _mockConfigService, _mockBotConfigurator, - _mockExecutor, spyService, _mockConfirmationProvider, _federatedCredentialService); + _mockExecutor, spyService, _mockConfirmationProvider, _federatedCredentialService, _mockPrerequisiteRunner, _mockAuthValidator); var args = new[] { "cleanup", "blueprint", "--config", "test.json" }; // Act @@ -474,7 +486,7 @@ public async Task CleanupBlueprint_WhenBlueprintDeletionFailsWithInstances_LogsW var command = CleanupCommand.CreateCommand( _mockLogger, _mockConfigService, _mockBotConfigurator, - _mockExecutor, spyService, _mockConfirmationProvider, _federatedCredentialService); + _mockExecutor, spyService, _mockConfirmationProvider, _federatedCredentialService, _mockPrerequisiteRunner, _mockAuthValidator); var args = new[] { "cleanup", "blueprint", "--config", "test.json" }; // Act @@ -554,7 +566,7 @@ public async Task Cleanup_WhenUserDeclinesInitialConfirmation_ShouldAbortWithout // User declines the initial "Are you sure?" confirmation _mockConfirmationProvider.ConfirmAsync(Arg.Any()).Returns(false); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService, _mockPrerequisiteRunner, _mockAuthValidator); var args = new[] { "cleanup", "--config", "test.json" }; // Act @@ -582,7 +594,7 @@ public async Task Cleanup_WhenUserConfirmsButDoesNotTypeDelete_ShouldAbortWithou _mockConfirmationProvider.ConfirmAsync(Arg.Any()).Returns(true); _mockConfirmationProvider.ConfirmWithTypedResponseAsync(Arg.Any(), "DELETE").Returns(false); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService, _mockPrerequisiteRunner, _mockAuthValidator); var args = new[] { "cleanup", "--config", "test.json" }; // Act @@ -606,7 +618,7 @@ public async Task Cleanup_ShouldCallConfirmationProviderWithCorrectPrompts() var config = CreateValidConfig(); _mockConfigService.LoadAsync(Arg.Any(), Arg.Any()).Returns(config); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService, _mockPrerequisiteRunner, _mockAuthValidator); var args = new[] { "cleanup", "--config", "test.json" }; // Act @@ -632,7 +644,9 @@ public void CleanupCommand_ShouldAcceptConfirmationProviderParameter() _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, - _federatedCredentialService); + _federatedCredentialService, + _mockPrerequisiteRunner, + _mockAuthValidator); command.Should().NotBeNull(); command.Name.Should().Be("cleanup"); @@ -645,7 +659,7 @@ public void CleanupCommand_ShouldAcceptConfirmationProviderParameter() public void CleanupBlueprint_ShouldHaveEndpointOnlyOption() { // Arrange & Act - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService, _mockPrerequisiteRunner, _mockAuthValidator); var blueprintCommand = command.Subcommands.First(sc => sc.Name == "blueprint"); // Assert @@ -666,7 +680,7 @@ public async Task CleanupBlueprint_WithEndpointOnly_ShouldOnlyDeleteMessagingEnd _mockBotConfigurator.DeleteEndpointWithAgentBlueprintAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) .Returns(true); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService, _mockPrerequisiteRunner, _mockAuthValidator); var args = new[] { "cleanup", "blueprint", "--endpoint-only", "--config", "test.json" }; // Simulate user confirmation with y @@ -725,7 +739,7 @@ public async Task CleanupBlueprint_WithEndpointOnlyAndNoBlueprintId_ShouldLogErr }; _mockConfigService.LoadAsync(Arg.Any(), Arg.Any()).Returns(config); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService, _mockPrerequisiteRunner, _mockAuthValidator); var args = new[] { "cleanup", "blueprint", "--endpoint-only", "--config", "test.json" }; // Act @@ -763,7 +777,7 @@ public async Task CleanupBlueprint_WithEndpointOnlyAndNoBotName_ShouldLogInfo() }; _mockConfigService.LoadAsync(Arg.Any(), Arg.Any()).Returns(config); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService, _mockPrerequisiteRunner, _mockAuthValidator); var args = new[] { "cleanup", "blueprint", "--endpoint-only", "--config", "test.json" }; // Act @@ -803,7 +817,7 @@ public async Task CleanupBlueprint_WithEndpointOnlyAndMissingLocation_ShouldNotC }; _mockConfigService.LoadAsync(Arg.Any(), Arg.Any()).Returns(config); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService, _mockPrerequisiteRunner, _mockAuthValidator); var args = new[] { "cleanup", "blueprint", "--endpoint-only", "--config", "test.json" }; var originalIn = Console.In; @@ -844,7 +858,7 @@ public async Task CleanupBlueprint_WithEndpointOnlyAndApiException_ShouldHandleG _mockBotConfigurator.DeleteEndpointWithAgentBlueprintAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) .Returns(Task.FromException(new InvalidOperationException("API connection failed"))); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService, _mockPrerequisiteRunner, _mockAuthValidator); var args = new[] { "cleanup", "blueprint", "--endpoint-only", "--config", "test.json" }; var originalIn = Console.In; @@ -898,7 +912,7 @@ public async Task CleanupBlueprint_WithEndpointOnlyAndWhitespaceBlueprint_Should }; _mockConfigService.LoadAsync(Arg.Any(), Arg.Any()).Returns(config); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService, _mockPrerequisiteRunner, _mockAuthValidator); var args = new[] { "cleanup", "blueprint", "--endpoint-only", "--config", "test.json" }; // Act @@ -925,7 +939,7 @@ public async Task CleanupBlueprint_WithEndpointOnlyAndInvalidInput_ShouldCancelC _mockBotConfigurator.DeleteEndpointWithAgentBlueprintAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) .Returns(true); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService, _mockPrerequisiteRunner, _mockAuthValidator); var args = new[] { "cleanup", "blueprint", "--endpoint-only", "--config", "test.json" }; var originalIn = Console.In; @@ -964,7 +978,7 @@ public async Task CleanupBlueprint_WithEndpointOnlyAndNoResponse_ShouldCancelCle _mockBotConfigurator.DeleteEndpointWithAgentBlueprintAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) .Returns(true); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService, _mockPrerequisiteRunner, _mockAuthValidator); var args = new[] { "cleanup", "blueprint", "--endpoint-only", "--config", "test.json" }; var originalIn = Console.In; @@ -1003,7 +1017,7 @@ public async Task CleanupBlueprint_WithEndpointOnlyAndEmptyInput_ShouldCancelCle _mockBotConfigurator.DeleteEndpointWithAgentBlueprintAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) .Returns(true); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService, _mockPrerequisiteRunner, _mockAuthValidator); var args = new[] { "cleanup", "blueprint", "--endpoint-only", "--config", "test.json" }; var originalIn = Console.In; diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CreateInstanceCommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CreateInstanceCommandTests.cs index e8654510..488806b7 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CreateInstanceCommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CreateInstanceCommandTests.cs @@ -21,18 +21,16 @@ public class CreateInstanceCommandTests private readonly CommandExecutor _mockExecutor; private readonly IBotConfigurator _mockBotConfigurator; private readonly GraphApiService _mockGraphApiService; - private readonly IAzureValidator _mockAzureValidator; public CreateInstanceCommandTests() { _mockLogger = Substitute.For>(); - + // Use NullLogger instead of console logger to avoid I/O bottleneck _mockConfigService = Substitute.ForPartsOf(NullLogger.Instance); _mockExecutor = Substitute.ForPartsOf(NullLogger.Instance); _mockBotConfigurator = Substitute.For(); _mockGraphApiService = Substitute.ForPartsOf(NullLogger.Instance, _mockExecutor); - _mockAzureValidator = Substitute.For(); } [Fact] @@ -44,8 +42,7 @@ public void CreateInstanceCommand_Should_Not_Have_Identity_Subcommand_Due_To_Dep _mockConfigService, _mockExecutor, _mockBotConfigurator, - _mockGraphApiService, - _mockAzureValidator); + _mockGraphApiService); // Act var identitySubcommand = command.Subcommands.FirstOrDefault(c => c.Name == "identity"); @@ -63,8 +60,7 @@ public void CreateInstanceCommand_Should_Not_Have_Licenses_Subcommand_Due_To_Dep _mockConfigService, _mockExecutor, _mockBotConfigurator, - _mockGraphApiService, - _mockAzureValidator); + _mockGraphApiService); // Act var licensesSubcommand = command.Subcommands.FirstOrDefault(c => c.Name == "licenses"); @@ -82,8 +78,7 @@ public void CreateInstanceCommand_Should_Have_Handler_For_Complete_Instance_Crea _mockConfigService, _mockExecutor, _mockBotConfigurator, - _mockGraphApiService, - _mockAzureValidator); + _mockGraphApiService); // Act & Assert - Main command should have handler for running all steps Assert.NotNull(command.Handler); @@ -98,8 +93,7 @@ public void CreateInstanceCommand_Should_Log_Deprecation_Error() _mockConfigService, _mockExecutor, _mockBotConfigurator, - _mockGraphApiService, - _mockAzureValidator); + _mockGraphApiService); // Act - Command should be created successfully // Assert - Command structure is valid diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DeployCommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DeployCommandTests.cs index 261e075a..11bb555a 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DeployCommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DeployCommandTests.cs @@ -9,6 +9,7 @@ using Microsoft.Agents.A365.DevTools.Cli.Commands; using Microsoft.Agents.A365.DevTools.Cli.Models; using Microsoft.Agents.A365.DevTools.Cli.Services; +using Microsoft.Extensions.Logging.Abstractions; using NSubstitute; using Xunit; @@ -23,7 +24,8 @@ public class DeployCommandTests private readonly ConfigService _mockConfigService; private readonly CommandExecutor _mockExecutor; private readonly DeploymentService _mockDeploymentService; - private readonly IAzureValidator _mockAzureValidator; + private readonly IPrerequisiteRunner _mockPrerequisiteRunner; + private readonly AzureAuthValidator _mockAuthValidator; private readonly GraphApiService _mockGraphApiService; private readonly AgentBlueprintService _mockBlueprintService; @@ -52,7 +54,8 @@ public DeployCommandTests() mockNodeLogger, mockPythonLogger); - _mockAzureValidator = Substitute.For(); + _mockPrerequisiteRunner = Substitute.For(); + _mockAuthValidator = Substitute.ForPartsOf(NullLogger.Instance, _mockExecutor); _mockGraphApiService = Substitute.ForPartsOf(Substitute.For>(), _mockExecutor); _mockBlueprintService = Substitute.ForPartsOf(Substitute.For>(), _mockGraphApiService); } @@ -66,7 +69,8 @@ public void UpdateCommand_Should_Not_Have_Atg_Subcommand() _mockConfigService, _mockExecutor, _mockDeploymentService, - _mockAzureValidator, + _mockPrerequisiteRunner, + _mockAuthValidator, _mockGraphApiService, _mockBlueprintService); // Act @@ -85,7 +89,8 @@ public void UpdateCommand_Should_Have_Config_Option_With_Default() _mockConfigService, _mockExecutor, _mockDeploymentService, - _mockAzureValidator, + _mockPrerequisiteRunner, + _mockAuthValidator, _mockGraphApiService, _mockBlueprintService); // Act @@ -105,7 +110,8 @@ public void UpdateCommand_Should_Have_Verbose_Option() _mockConfigService, _mockExecutor, _mockDeploymentService, - _mockAzureValidator, + _mockPrerequisiteRunner, + _mockAuthValidator, _mockGraphApiService, _mockBlueprintService); // Act diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupCommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupCommandTests.cs index ee4fcd94..660ba925 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupCommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupCommandTests.cs @@ -5,6 +5,7 @@ using Microsoft.Agents.A365.DevTools.Cli.Commands; using Microsoft.Agents.A365.DevTools.Cli.Models; using Microsoft.Agents.A365.DevTools.Cli.Services; +using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Logging; using NSubstitute; using System.CommandLine; @@ -24,7 +25,9 @@ public class SetupCommandTests private readonly CommandExecutor _mockExecutor; private readonly DeploymentService _mockDeploymentService; private readonly IBotConfigurator _mockBotConfigurator; - private readonly IAzureValidator _mockAzureValidator; + private readonly IPrerequisiteRunner _mockPrerequisiteRunner; + private readonly AzureAuthValidator _mockAuthValidator; + private readonly IAzureEnvironmentValidator _mockEnvironmentValidator; private readonly AzureWebAppCreator _mockWebAppCreator; private readonly PlatformDetector _mockPlatformDetector; private readonly GraphApiService _mockGraphApiService; @@ -53,7 +56,9 @@ public SetupCommandTests() mockNodeLogger, mockPythonLogger); _mockBotConfigurator = Substitute.For(); - _mockAzureValidator = Substitute.For(); + _mockPrerequisiteRunner = Substitute.For(); + _mockAuthValidator = Substitute.ForPartsOf(NullLogger.Instance, _mockExecutor); + _mockEnvironmentValidator = Substitute.For(); _mockWebAppCreator = Substitute.ForPartsOf(Substitute.For>()); _mockGraphApiService = Substitute.For(); _mockBlueprintService = Substitute.ForPartsOf(Substitute.For>(), _mockGraphApiService); @@ -85,9 +90,11 @@ public async Task SetupAllCommand_DryRun_ValidConfig_OnlyValidatesConfig() _mockConfigService, _mockExecutor, _mockDeploymentService, - _mockBotConfigurator, - _mockAzureValidator, - _mockWebAppCreator, + _mockBotConfigurator, + _mockPrerequisiteRunner, + _mockAuthValidator, + _mockEnvironmentValidator, + _mockWebAppCreator, _mockPlatformDetector, _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService, _mockClientAppValidator); @@ -102,7 +109,6 @@ public async Task SetupAllCommand_DryRun_ValidConfig_OnlyValidatesConfig() // Dry-run mode does not load config or call Azure/Bot services - it just displays what would be done await _mockConfigService.DidNotReceiveWithAnyArgs().LoadAsync(Arg.Any(), Arg.Any()); - await _mockAzureValidator.DidNotReceiveWithAnyArgs().ValidateAllAsync(default!); await _mockBotConfigurator.DidNotReceiveWithAnyArgs().CreateEndpointWithAgentBlueprintAsync(default!, default!, default!, default!, default!); } @@ -132,9 +138,11 @@ public async Task SetupAllCommand_SkipInfrastructure_SkipsInfrastructureStep() _mockConfigService, _mockExecutor, _mockDeploymentService, - _mockBotConfigurator, - _mockAzureValidator, - _mockWebAppCreator, + _mockBotConfigurator, + _mockPrerequisiteRunner, + _mockAuthValidator, + _mockEnvironmentValidator, + _mockWebAppCreator, _mockPlatformDetector, _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService, _mockClientAppValidator); @@ -160,9 +168,11 @@ public void SetupCommand_HasRequiredSubcommands() _mockConfigService, _mockExecutor, _mockDeploymentService, - _mockBotConfigurator, - _mockAzureValidator, - _mockWebAppCreator, + _mockBotConfigurator, + _mockPrerequisiteRunner, + _mockAuthValidator, + _mockEnvironmentValidator, + _mockWebAppCreator, _mockPlatformDetector, _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService, _mockClientAppValidator); @@ -185,9 +195,11 @@ public void SetupCommand_PermissionsSubcommand_HasMcpAndBotSubcommands() _mockConfigService, _mockExecutor, _mockDeploymentService, - _mockBotConfigurator, - _mockAzureValidator, - _mockWebAppCreator, + _mockBotConfigurator, + _mockPrerequisiteRunner, + _mockAuthValidator, + _mockEnvironmentValidator, + _mockWebAppCreator, _mockPlatformDetector, _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService, _mockClientAppValidator); @@ -213,9 +225,11 @@ public void SetupCommand_ErrorMessages_ShouldBeInformativeAndActionable() _mockConfigService, _mockExecutor, _mockDeploymentService, - _mockBotConfigurator, - _mockAzureValidator, - _mockWebAppCreator, + _mockBotConfigurator, + _mockPrerequisiteRunner, + _mockAuthValidator, + _mockEnvironmentValidator, + _mockWebAppCreator, _mockPlatformDetector, _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService, _mockClientAppValidator); @@ -259,8 +273,10 @@ public async Task InfrastructureSubcommand_DryRun_CompletesSuccessfully() _mockConfigService, _mockExecutor, _mockDeploymentService, - _mockBotConfigurator, - _mockAzureValidator, + _mockBotConfigurator, + _mockPrerequisiteRunner, + _mockAuthValidator, + _mockEnvironmentValidator, _mockWebAppCreator, _mockPlatformDetector, _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService, _mockClientAppValidator); @@ -303,7 +319,9 @@ public async Task BlueprintSubcommand_DryRun_CompletesSuccessfully() _mockExecutor, _mockDeploymentService, _mockBotConfigurator, - _mockAzureValidator, + _mockPrerequisiteRunner, + _mockAuthValidator, + _mockEnvironmentValidator, _mockWebAppCreator, _mockPlatformDetector, _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService, _mockClientAppValidator); @@ -345,7 +363,9 @@ public async Task RequirementsSubcommand_ValidConfig_CompletesSuccessfully() _mockExecutor, _mockDeploymentService, _mockBotConfigurator, - _mockAzureValidator, + _mockPrerequisiteRunner, + _mockAuthValidator, + _mockEnvironmentValidator, _mockWebAppCreator, _mockPlatformDetector, _mockGraphApiService, @@ -389,7 +409,9 @@ public async Task RequirementsSubcommand_WithCategoryFilter_RunsFilteredChecks() _mockExecutor, _mockDeploymentService, _mockBotConfigurator, - _mockAzureValidator, + _mockPrerequisiteRunner, + _mockAuthValidator, + _mockEnvironmentValidator, _mockWebAppCreator, _mockPlatformDetector, _mockGraphApiService, diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SubcommandValidationTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SubcommandValidationTests.cs index 95077d95..df5f577e 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SubcommandValidationTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SubcommandValidationTests.cs @@ -6,8 +6,8 @@ using Microsoft.Agents.A365.DevTools.Cli.Constants; using Microsoft.Agents.A365.DevTools.Cli.Helpers; using Microsoft.Agents.A365.DevTools.Cli.Models; -using Microsoft.Agents.A365.DevTools.Cli.Services; using Microsoft.Agents.A365.DevTools.Cli.Services.Requirements; +using Microsoft.Agents.A365.DevTools.Cli.Services.Requirements.RequirementChecks; using Microsoft.Agents.A365.DevTools.Cli.Tests.TestHelpers; using Microsoft.Extensions.Logging; using NSubstitute; @@ -21,21 +21,20 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Commands; /// public class SubcommandValidationTests { - private readonly IAzureValidator _mockAzureValidator; - private readonly IClientAppValidator _mockClientAppValidator; + private readonly ILogger _mockLogger; public SubcommandValidationTests() { - _mockAzureValidator = Substitute.For(); - _mockClientAppValidator = Substitute.For(); + _mockLogger = Substitute.For(); } - #region InfrastructureSubcommand Validation Tests + #region InfrastructureRequirementCheck Validation Tests [Fact] public async Task InfrastructureSubcommand_WithValidConfig_PassesValidation() { // Arrange + var check = new InfrastructureRequirementCheck(); var config = new Agent365Config { NeedDeployment = true, @@ -44,20 +43,21 @@ public async Task InfrastructureSubcommand_WithValidConfig_PassesValidation() AppServicePlanName = "test-plan", WebAppName = "test-webapp", Location = "westus", - AppServicePlanSku = "F1" // Use F1 to avoid B1 quota warning + AppServicePlanSku = "F1" }; // Act - var errors = await InfrastructureSubcommand.ValidateAsync(config, _mockAzureValidator); + var result = await check.CheckAsync(config, _mockLogger); // Assert - errors.Should().BeEmpty(); + result.Passed.Should().BeTrue(); } [Fact] public async Task InfrastructureSubcommand_WithMissingSubscriptionId_FailsValidation() { // Arrange + var check = new InfrastructureRequirementCheck(); var config = new Agent365Config { NeedDeployment = true, @@ -66,21 +66,22 @@ public async Task InfrastructureSubcommand_WithMissingSubscriptionId_FailsValida AppServicePlanName = "test-plan", WebAppName = "test-webapp", Location = "westus", - AppServicePlanSku = "F1" // Use F1 to avoid B1 quota warning + AppServicePlanSku = "F1" }; // Act - var errors = await InfrastructureSubcommand.ValidateAsync(config, _mockAzureValidator); + var result = await check.CheckAsync(config, _mockLogger); // Assert - errors.Should().ContainSingle() - .Which.Should().Contain("subscriptionId"); + result.Passed.Should().BeFalse(); + result.ErrorMessage.Should().Contain("subscriptionId"); } [Fact] public async Task InfrastructureSubcommand_WithMissingResourceGroup_FailsValidation() { // Arrange + var check = new InfrastructureRequirementCheck(); var config = new Agent365Config { NeedDeployment = true, @@ -89,21 +90,22 @@ public async Task InfrastructureSubcommand_WithMissingResourceGroup_FailsValidat AppServicePlanName = "test-plan", WebAppName = "test-webapp", Location = "westus", - AppServicePlanSku = "F1" // Use F1 to avoid B1 quota warning + AppServicePlanSku = "F1" }; // Act - var errors = await InfrastructureSubcommand.ValidateAsync(config, _mockAzureValidator); + var result = await check.CheckAsync(config, _mockLogger); // Assert - errors.Should().ContainSingle() - .Which.Should().Contain("resourceGroup"); + result.Passed.Should().BeFalse(); + result.ErrorMessage.Should().Contain("resourceGroup"); } [Fact] public async Task InfrastructureSubcommand_WithMultipleMissingFields_ReturnsAllErrors() { // Arrange + var check = new InfrastructureRequirementCheck(); var config = new Agent365Config { NeedDeployment = true, @@ -112,23 +114,24 @@ public async Task InfrastructureSubcommand_WithMultipleMissingFields_ReturnsAllE AppServicePlanName = "", WebAppName = "test-webapp", Location = "westus", - AppServicePlanSku = "F1" // Use F1 to avoid B1 quota warning + AppServicePlanSku = "F1" }; // Act - var errors = await InfrastructureSubcommand.ValidateAsync(config, _mockAzureValidator); + var result = await check.CheckAsync(config, _mockLogger); // Assert - errors.Should().HaveCount(3); - errors.Should().Contain(e => e.Contains("subscriptionId")); - errors.Should().Contain(e => e.Contains("resourceGroup")); - errors.Should().Contain(e => e.Contains("appServicePlanName")); + result.Passed.Should().BeFalse(); + result.ErrorMessage.Should().Contain("subscriptionId"); + result.ErrorMessage.Should().Contain("resourceGroup"); + result.ErrorMessage.Should().Contain("appServicePlanName"); } [Fact] public async Task InfrastructureSubcommand_WhenNeedDeploymentFalse_SkipsValidation() { // Arrange + var check = new InfrastructureRequirementCheck(); var config = new Agent365Config { NeedDeployment = false, @@ -140,16 +143,17 @@ public async Task InfrastructureSubcommand_WhenNeedDeploymentFalse_SkipsValidati }; // Act - var errors = await InfrastructureSubcommand.ValidateAsync(config, _mockAzureValidator); + var result = await check.CheckAsync(config, _mockLogger); // Assert - errors.Should().BeEmpty(); + result.Passed.Should().BeTrue(); } [Fact] public async Task InfrastructureSubcommand_WithInvalidSku_FailsValidation() { // Arrange + var check = new InfrastructureRequirementCheck(); var config = new Agent365Config { NeedDeployment = true, @@ -162,17 +166,18 @@ public async Task InfrastructureSubcommand_WithInvalidSku_FailsValidation() }; // Act - var errors = await InfrastructureSubcommand.ValidateAsync(config, _mockAzureValidator); + var result = await check.CheckAsync(config, _mockLogger); // Assert - errors.Should().ContainSingle() - .Which.Should().Contain("Invalid appServicePlanSku"); + result.Passed.Should().BeFalse(); + result.ErrorMessage.Should().Contain("Invalid appServicePlanSku"); } [Fact] public async Task InfrastructureSubcommand_WithB1Sku_PassesValidation() { // Arrange + var check = new InfrastructureRequirementCheck(); var config = new Agent365Config { NeedDeployment = true, @@ -185,10 +190,10 @@ public async Task InfrastructureSubcommand_WithB1Sku_PassesValidation() }; // Act - var errors = await InfrastructureSubcommand.ValidateAsync(config, _mockAzureValidator); + var result = await check.CheckAsync(config, _mockLogger); - // Assert - B1 quota warning is now logged at execution time, not during validation - errors.Should().BeEmpty(); + // Assert + result.Passed.Should().BeTrue(); } [Theory] @@ -201,6 +206,7 @@ public async Task InfrastructureSubcommand_WithB1Sku_PassesValidation() public async Task InfrastructureSubcommand_WithValidSku_PassesValidationOrWarning(string sku) { // Arrange + var check = new InfrastructureRequirementCheck(); var config = new Agent365Config { NeedDeployment = true, @@ -213,48 +219,10 @@ public async Task InfrastructureSubcommand_WithValidSku_PassesValidationOrWarnin }; // Act - var errors = await InfrastructureSubcommand.ValidateAsync(config, _mockAzureValidator); - - // Assert - All valid SKUs pass validation (B1 quota warning is logged at execution time) - errors.Should().BeEmpty(); - } - - #endregion - - #region BlueprintSubcommand Validation Tests - - [Fact] - public async Task BlueprintSubcommand_WithValidConfig_PassesValidation() - { - // Arrange - var config = new Agent365Config - { - ClientAppId = "12345678-1234-1234-1234-123456789012" - }; - - // Act - var errors = await BlueprintSubcommand.ValidateAsync(config, _mockAzureValidator, _mockClientAppValidator); - - // Assert - errors.Should().BeEmpty(); - } - - [Fact] - public async Task BlueprintSubcommand_WithMissingClientAppId_FailsValidation() - { - // Arrange - var config = new Agent365Config - { - ClientAppId = "" - }; - - // Act - var errors = await BlueprintSubcommand.ValidateAsync(config, _mockAzureValidator, _mockClientAppValidator); + var result = await check.CheckAsync(config, _mockLogger); // Assert - errors.Should().HaveCountGreaterThan(0); - errors.Should().Contain(e => e.Contains("clientAppId")); - errors.Should().Contain(e => e.Contains("learn.microsoft.com")); + result.Passed.Should().BeTrue(); } #endregion diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/PrerequisiteRunnerTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/PrerequisiteRunnerTests.cs new file mode 100644 index 00000000..cffdbd31 --- /dev/null +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/PrerequisiteRunnerTests.cs @@ -0,0 +1,166 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using FluentAssertions; +using Microsoft.Agents.A365.DevTools.Cli.Models; +using Microsoft.Agents.A365.DevTools.Cli.Services; +using Microsoft.Agents.A365.DevTools.Cli.Services.Requirements; +using Microsoft.Agents.A365.DevTools.Cli.Tests.TestHelpers; +using Microsoft.Extensions.Logging; +using NSubstitute; +using Xunit; + +namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Services; + +/// +/// Unit tests for PrerequisiteRunner +/// +public class PrerequisiteRunnerTests +{ + private readonly ILogger _mockLogger; + private readonly Agent365Config _config; + + public PrerequisiteRunnerTests() + { + _mockLogger = Substitute.For(); + _config = new Agent365Config { TenantId = "test-tenant", SubscriptionId = "test-sub" }; + } + + [Fact] + public async Task RunAsync_WithEmptyChecks_ShouldReturnTrue() + { + // Arrange + var runner = new PrerequisiteRunner(); + var checks = new List(); + + // Act + var result = await runner.RunAsync(checks, _config, _mockLogger); + + // Assert + result.Should().BeTrue(); + } + + [Fact] + public async Task RunAsync_WithAllPassingChecks_ShouldReturnTrue() + { + // Arrange + var runner = new PrerequisiteRunner(); + var checks = new List + { + new AlwaysPassRequirementCheck(), + new AlwaysPassRequirementCheck() + }; + + // Act + var result = await runner.RunAsync(checks, _config, _mockLogger); + + // Assert + result.Should().BeTrue(); + } + + [Fact] + public async Task RunAsync_WithOneFailingCheck_ShouldReturnFalse() + { + // Arrange + var runner = new PrerequisiteRunner(); + var checks = new List + { + new AlwaysPassRequirementCheck(), + new AlwaysFailRequirementCheck() + }; + + // Act + var result = await runner.RunAsync(checks, _config, _mockLogger); + + // Assert + result.Should().BeFalse(); + } + + [Fact] + public async Task RunAsync_WithFailingCheck_ShouldLogError() + { + // Arrange + var runner = new PrerequisiteRunner(); + var checks = new List { new AlwaysFailRequirementCheck() }; + + // Act + await runner.RunAsync(checks, _config, _mockLogger); + + // Assert - should log an error for the failing check + _mockLogger.Received().Log( + LogLevel.Error, + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any>()); + } + + [Fact] + public async Task RunAsync_WithMultipleFailingChecks_ShouldReturnFalseAndLogAll() + { + // Arrange + var runner = new PrerequisiteRunner(); + var checks = new List + { + new AlwaysFailRequirementCheck(), + new AlwaysFailRequirementCheck() + }; + + // Act + var result = await runner.RunAsync(checks, _config, _mockLogger); + + // Assert + result.Should().BeFalse(); + } + + [Fact] + public async Task RunAsync_WithWarningCheck_ShouldReturnTrueAndLogWarning() + { + // Arrange + var runner = new PrerequisiteRunner(); + var mockCheck = Substitute.For(); + mockCheck.Name.Returns("Warning Check"); + mockCheck.CheckAsync(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(RequirementCheckResult.Warning("This is a warning", "Warning details")); + + var checks = new List { mockCheck }; + + // Act + var result = await runner.RunAsync(checks, _config, _mockLogger); + + // Assert + result.Should().BeTrue("a warning does not block execution"); + _mockLogger.Received().Log( + LogLevel.Warning, + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any>()); + } + + [Fact] + public async Task RunAsync_ChecksAreRunInOrder() + { + // Arrange + var runner = new PrerequisiteRunner(); + var executionOrder = new List(); + + var check1 = Substitute.For(); + check1.Name.Returns("Check1"); + check1.CheckAsync(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(_ => { executionOrder.Add("Check1"); return Task.FromResult(RequirementCheckResult.Success()); }); + + var check2 = Substitute.For(); + check2.Name.Returns("Check2"); + check2.CheckAsync(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(_ => { executionOrder.Add("Check2"); return Task.FromResult(RequirementCheckResult.Success()); }); + + var checks = new List { check1, check2 }; + + // Act + await runner.RunAsync(checks, _config, _mockLogger); + + // Assert + executionOrder.Should().Equal("Check1", "Check2"); + } +} diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Requirements/AzureAuthRequirementCheckTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Requirements/AzureAuthRequirementCheckTests.cs new file mode 100644 index 00000000..e803b4e0 --- /dev/null +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Requirements/AzureAuthRequirementCheckTests.cs @@ -0,0 +1,131 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using FluentAssertions; +using Microsoft.Agents.A365.DevTools.Cli.Models; +using Microsoft.Agents.A365.DevTools.Cli.Services; +using Microsoft.Agents.A365.DevTools.Cli.Services.Requirements.RequirementChecks; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; +using Xunit; + +namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Services.Requirements; + +/// +/// Unit tests for AzureAuthRequirementCheck +/// +public class AzureAuthRequirementCheckTests +{ + private readonly AzureAuthValidator _mockAuthValidator; + private readonly ILogger _mockLogger; + + public AzureAuthRequirementCheckTests() + { + var mockExecutor = Substitute.ForPartsOf(NullLogger.Instance); + _mockAuthValidator = Substitute.ForPartsOf(NullLogger.Instance, mockExecutor); + _mockLogger = Substitute.For(); + } + + [Fact] + public async Task CheckAsync_WhenAuthenticationSucceeds_ShouldReturnSuccess() + { + // Arrange + var check = new AzureAuthRequirementCheck(_mockAuthValidator); + var config = new Agent365Config { SubscriptionId = "test-sub-id" }; + + _mockAuthValidator.ValidateAuthenticationAsync(Arg.Any()) + .Returns(true); + + // Act + var result = await check.CheckAsync(config, _mockLogger); + + // Assert + result.Should().NotBeNull(); + result.Passed.Should().BeTrue(); + result.ErrorMessage.Should().BeNullOrEmpty(); + } + + [Fact] + public async Task CheckAsync_WhenAuthenticationFails_ShouldReturnFailure() + { + // Arrange + var check = new AzureAuthRequirementCheck(_mockAuthValidator); + var config = new Agent365Config { SubscriptionId = "test-sub-id" }; + + _mockAuthValidator.ValidateAuthenticationAsync(Arg.Any()) + .Returns(false); + + // Act + var result = await check.CheckAsync(config, _mockLogger); + + // Assert + result.Should().NotBeNull(); + result.Passed.Should().BeFalse(); + result.ErrorMessage.Should().Contain("Azure CLI authentication failed"); + result.ResolutionGuidance.Should().Contain("az login"); + } + + [Fact] + public async Task CheckAsync_ShouldPassSubscriptionIdToValidator() + { + // Arrange + var check = new AzureAuthRequirementCheck(_mockAuthValidator); + var config = new Agent365Config { SubscriptionId = "specific-sub-id" }; + + _mockAuthValidator.ValidateAuthenticationAsync(Arg.Any()) + .Returns(true); + + // Act + await check.CheckAsync(config, _mockLogger); + + // Assert + await _mockAuthValidator.Received(1).ValidateAuthenticationAsync("specific-sub-id"); + } + + [Fact] + public async Task CheckAsync_WithEmptySubscriptionId_ShouldPassEmptyStringToValidator() + { + // Arrange + var check = new AzureAuthRequirementCheck(_mockAuthValidator); + var config = new Agent365Config(); + + _mockAuthValidator.ValidateAuthenticationAsync(Arg.Any()) + .Returns(true); + + // Act + await check.CheckAsync(config, _mockLogger); + + // Assert + await _mockAuthValidator.Received(1).ValidateAuthenticationAsync(string.Empty); + } + + [Fact] + public void Metadata_ShouldHaveCorrectName() + { + // Arrange + var check = new AzureAuthRequirementCheck(_mockAuthValidator); + + // Act & Assert + check.Name.Should().Be("Azure Authentication"); + } + + [Fact] + public void Metadata_ShouldHaveCorrectCategory() + { + // Arrange + var check = new AzureAuthRequirementCheck(_mockAuthValidator); + + // Act & Assert + check.Category.Should().Be("Azure"); + } + + [Fact] + public void Constructor_WithNullValidator_ShouldThrowArgumentNullException() + { + // Act & Assert + var act = () => new AzureAuthRequirementCheck(null!); + act.Should().Throw() + .WithParameterName("authValidator"); + } +} diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Requirements/InfrastructureRequirementCheckTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Requirements/InfrastructureRequirementCheckTests.cs new file mode 100644 index 00000000..b8055191 --- /dev/null +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Requirements/InfrastructureRequirementCheckTests.cs @@ -0,0 +1,245 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using FluentAssertions; +using Microsoft.Agents.A365.DevTools.Cli.Models; +using Microsoft.Agents.A365.DevTools.Cli.Services.Requirements.RequirementChecks; +using Microsoft.Extensions.Logging; +using NSubstitute; +using Xunit; + +namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Services.Requirements; + +/// +/// Unit tests for InfrastructureRequirementCheck +/// +public class InfrastructureRequirementCheckTests +{ + private readonly ILogger _mockLogger; + + public InfrastructureRequirementCheckTests() + { + _mockLogger = Substitute.For(); + } + + [Fact] + public async Task CheckAsync_WhenNeedDeploymentFalse_ShouldReturnSuccess() + { + // Arrange + var check = new InfrastructureRequirementCheck(); + var config = new Agent365Config + { + NeedDeployment = false, + SubscriptionId = "", + ResourceGroup = "", + AppServicePlanName = "", + WebAppName = "", + Location = "" + }; + + // Act + var result = await check.CheckAsync(config, _mockLogger); + + // Assert + result.Passed.Should().BeTrue(); + result.ErrorMessage.Should().BeNullOrEmpty(); + } + + [Fact] + public async Task CheckAsync_WithAllRequiredFields_ShouldReturnSuccess() + { + // Arrange + var check = new InfrastructureRequirementCheck(); + var config = new Agent365Config + { + NeedDeployment = true, + SubscriptionId = "test-sub-id", + ResourceGroup = "test-rg", + AppServicePlanName = "test-plan", + WebAppName = "test-webapp", + Location = "eastus", + AppServicePlanSku = "B1" + }; + + // Act + var result = await check.CheckAsync(config, _mockLogger); + + // Assert + result.Passed.Should().BeTrue(); + result.ErrorMessage.Should().BeNullOrEmpty(); + } + + [Fact] + public async Task CheckAsync_WithMissingSubscriptionId_ShouldReturnFailure() + { + // Arrange + var check = new InfrastructureRequirementCheck(); + var config = new Agent365Config + { + NeedDeployment = true, + SubscriptionId = "", + ResourceGroup = "test-rg", + AppServicePlanName = "test-plan", + WebAppName = "test-webapp", + Location = "eastus", + AppServicePlanSku = "B1" + }; + + // Act + var result = await check.CheckAsync(config, _mockLogger); + + // Assert + result.Passed.Should().BeFalse(); + result.ErrorMessage.Should().Contain("subscriptionId"); + result.ResolutionGuidance.Should().Contain("a365.config.json"); + } + + [Fact] + public async Task CheckAsync_WithMissingResourceGroup_ShouldReturnFailure() + { + // Arrange + var check = new InfrastructureRequirementCheck(); + var config = new Agent365Config + { + NeedDeployment = true, + SubscriptionId = "test-sub-id", + ResourceGroup = "", + AppServicePlanName = "test-plan", + WebAppName = "test-webapp", + Location = "eastus", + AppServicePlanSku = "B1" + }; + + // Act + var result = await check.CheckAsync(config, _mockLogger); + + // Assert + result.Passed.Should().BeFalse(); + result.ErrorMessage.Should().Contain("resourceGroup"); + } + + [Fact] + public async Task CheckAsync_WithMissingAppServicePlanName_ShouldReturnFailure() + { + // Arrange + var check = new InfrastructureRequirementCheck(); + var config = new Agent365Config + { + NeedDeployment = true, + SubscriptionId = "test-sub-id", + ResourceGroup = "test-rg", + AppServicePlanName = "", + WebAppName = "test-webapp", + Location = "eastus", + AppServicePlanSku = "B1" + }; + + // Act + var result = await check.CheckAsync(config, _mockLogger); + + // Assert + result.Passed.Should().BeFalse(); + result.ErrorMessage.Should().Contain("appServicePlanName"); + } + + [Fact] + public async Task CheckAsync_WithMultipleMissingFields_ShouldIncludeAllErrorsInMessage() + { + // Arrange + var check = new InfrastructureRequirementCheck(); + var config = new Agent365Config + { + NeedDeployment = true, + SubscriptionId = "", + ResourceGroup = "", + AppServicePlanName = "", + WebAppName = "test-webapp", + Location = "eastus", + AppServicePlanSku = "F1" + }; + + // Act + var result = await check.CheckAsync(config, _mockLogger); + + // Assert + result.Passed.Should().BeFalse(); + result.ErrorMessage.Should().Contain("subscriptionId"); + result.ErrorMessage.Should().Contain("resourceGroup"); + result.ErrorMessage.Should().Contain("appServicePlanName"); + } + + [Fact] + public async Task CheckAsync_WithInvalidSku_ShouldReturnFailure() + { + // Arrange + var check = new InfrastructureRequirementCheck(); + var config = new Agent365Config + { + NeedDeployment = true, + SubscriptionId = "test-sub-id", + ResourceGroup = "test-rg", + AppServicePlanName = "test-plan", + WebAppName = "test-webapp", + Location = "eastus", + AppServicePlanSku = "INVALID_SKU" + }; + + // Act + var result = await check.CheckAsync(config, _mockLogger); + + // Assert + result.Passed.Should().BeFalse(); + result.ErrorMessage.Should().Contain("Invalid appServicePlanSku"); + result.ErrorMessage.Should().Contain("INVALID_SKU"); + } + + [Theory] + [InlineData("F1")] + [InlineData("B1")] + [InlineData("B2")] + [InlineData("B3")] + [InlineData("S1")] + [InlineData("P1V2")] + [InlineData("P1V3")] + public async Task CheckAsync_WithValidSku_ShouldReturnSuccess(string sku) + { + // Arrange + var check = new InfrastructureRequirementCheck(); + var config = new Agent365Config + { + NeedDeployment = true, + SubscriptionId = "test-sub-id", + ResourceGroup = "test-rg", + AppServicePlanName = "test-plan", + WebAppName = "test-webapp", + Location = "eastus", + AppServicePlanSku = sku + }; + + // Act + var result = await check.CheckAsync(config, _mockLogger); + + // Assert + result.Passed.Should().BeTrue(); + } + + [Fact] + public void Metadata_ShouldHaveCorrectName() + { + // Arrange + var check = new InfrastructureRequirementCheck(); + + // Act & Assert + check.Name.Should().Be("Infrastructure Configuration"); + } + + [Fact] + public void Metadata_ShouldHaveCorrectCategory() + { + // Arrange + var check = new InfrastructureRequirementCheck(); + + // Act & Assert + check.Category.Should().Be("Configuration"); + } +} From 2cd8540f56523303d47a75014cfb638710d6f53f Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Fri, 6 Mar 2026 16:08:47 -0800 Subject: [PATCH 2/3] 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 --- .../Commands/CleanupCommand.cs | 5 +++-- .../Commands/CreateInstanceCommand.cs | 12 +++--------- .../Exceptions/ConfigFileNotFoundException.cs | 19 +++++++++---------- .../Services/ClientAppValidator.cs | 2 +- .../Services/ConfigurationWizardService.cs | 6 +++--- 5 files changed, 19 insertions(+), 25 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs index d1d3ac63..fcde04d6 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs @@ -6,6 +6,7 @@ using Microsoft.Extensions.Logging; using Microsoft.Agents.A365.DevTools.Cli.Constants; using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; +using Microsoft.Agents.A365.DevTools.Cli.Exceptions; using Microsoft.Agents.A365.DevTools.Cli.Services; using Microsoft.Agents.A365.DevTools.Cli.Services.Internal; using Microsoft.Agents.A365.DevTools.Cli.Services.Requirements.RequirementChecks; @@ -971,9 +972,9 @@ private static void PrintOrphanSummary( logger.LogInformation("Loaded configuration successfully from {ConfigFile}", configPath); return config; } - catch (FileNotFoundException ex) + catch (ConfigFileNotFoundException ex) { - logger.LogError("Configuration file not found: {Message}", ex.Message); + logger.LogError("Configuration file not found: {Message}", ex.IssueDescription); return null; } catch (Exception ex) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CreateInstanceCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CreateInstanceCommand.cs index bd44e63b..4134c95c 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CreateInstanceCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CreateInstanceCommand.cs @@ -4,6 +4,7 @@ using Microsoft.Agents.A365.DevTools.Cli.Constants; using Microsoft.Agents.A365.DevTools.Cli.Helpers; using Microsoft.Agents.A365.DevTools.Cli.Models; +using Microsoft.Agents.A365.DevTools.Cli.Exceptions; using Microsoft.Agents.A365.DevTools.Cli.Services; using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; using Microsoft.Extensions.Logging; @@ -499,16 +500,9 @@ private static Command CreateLicensesSubcommand( : await configService.LoadAsync(); return config; } - catch (FileNotFoundException ex) + catch (ConfigFileNotFoundException ex) { - logger.LogError("Configuration file not found: {Message}", ex.Message); - logger.LogInformation(""); - logger.LogInformation("To get started:"); - logger.LogInformation(" 1. Copy a365.config.example.json to a365.config.json"); - logger.LogInformation(" 2. Edit a365.config.json with your Azure tenant and subscription details"); - logger.LogInformation(" 3. Run 'a365 setup' to initialize your environment first"); - logger.LogInformation(" 4. Then run 'a365 createinstance' to create agent instances"); - logger.LogInformation(""); + logger.LogError("Configuration file not found: {Message}", ex.IssueDescription); return null; } catch (Exception ex) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Exceptions/ConfigFileNotFoundException.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Exceptions/ConfigFileNotFoundException.cs index d15f7906..e23fb618 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Exceptions/ConfigFileNotFoundException.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Exceptions/ConfigFileNotFoundException.cs @@ -1,26 +1,25 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -using System.IO; - namespace Microsoft.Agents.A365.DevTools.Cli.Exceptions; /// /// Exception thrown when the a365.config.json configuration file cannot be found. /// This is a USER ERROR - the file is missing or the command was run from the wrong directory. -/// Derives from FileNotFoundException so existing callers that catch FileNotFoundException -/// continue to work without changes. /// -public class ConfigFileNotFoundException : FileNotFoundException +public class ConfigFileNotFoundException : Agent365Exception { public ConfigFileNotFoundException(string configFilePath) : base( - message: $"Configuration file not found: {configFilePath}. " + - "Make sure you are running this command from your agent project directory. " + - "If you have not created a configuration file yet, run: a365 config init", - fileName: configFilePath) + errorCode: "CONFIG_NOT_FOUND", + issueDescription: $"Configuration file not found: {configFilePath}", + mitigationSteps: + [ + "Make sure you are running this command from your agent project directory.", + "If you have not created a configuration file yet, run: a365 config init" + ]) { } - public int ExitCode => 2; // Configuration error + public override int ExitCode => 2; // Configuration error } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/ClientAppValidator.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/ClientAppValidator.cs index bd724030..a62bac7a 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/ClientAppValidator.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/ClientAppValidator.cs @@ -292,7 +292,7 @@ private async Task EnsurePublicClientFlowsEnabledAsync( return; } - _logger.LogInformation("Enabling 'Allow public client flows' on app registration (required for device code authentication fallback on macOS/Linux)."); + _logger.LogInformation("Enabling 'Allow public client flows' on app registration (required for device code authentication fallback)."); _logger.LogInformation("Run 'a365 setup requirements' at any time to re-verify and auto-fix this setting."); var patchBody = "{\"isFallbackPublicClient\":true}"; diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/ConfigurationWizardService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/ConfigurationWizardService.cs index a7597662..bc796728 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/ConfigurationWizardService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/ConfigurationWizardService.cs @@ -338,8 +338,8 @@ private string PromptForDeploymentPath(Agent365Config? existingConfig) Console.WriteLine("This is used to detect your project type (.NET, Node.js, or Python)"); Console.WriteLine("and as the source directory for Azure App Service deployment."); Console.WriteLine(); - Console.WriteLine(" Use '.' if you are already running this command from your project folder."); - Console.WriteLine(@" Example: /home/user/my-agent or C:\Projects\my-agent"); + Console.WriteLine(" Absolute and relative paths are both accepted and will be resolved to a full path."); + Console.WriteLine(@" Example: /home/user/my-agent or C:\Projects\my-agent or ."); Console.WriteLine("================================================================="); Console.WriteLine(); @@ -371,7 +371,7 @@ private string PromptForDeploymentPath(Agent365Config? existingConfig) } } - return path; + return Path.GetFullPath(path); } private async Task<(string name, string? location)> PromptForResourceGroupAsync(Agent365Config? existingConfig) From 7257a565da3cffb46cb1ef363d176b7a1358cf13 Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Fri, 6 Mar 2026 18:37:21 -0800 Subject: [PATCH 3/3] 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 --- .../Commands/SetupCommand.cs | 2 +- .../SetupSubcommands/AllSubcommand.cs | 99 ++++--------------- .../SetupSubcommands/BlueprintSubcommand.cs | 26 ++--- .../RequirementsSubcommand.cs | 41 +++----- .../Services/PlatformDetector.cs | 8 +- .../Services/Requirements/RequirementCheck.cs | 29 ++---- .../AzureAuthRequirementCheck.cs | 10 +- .../ClientAppRequirementCheck.cs | 2 +- .../FrontierPreviewRequirementCheck.cs | 21 +--- .../LocationRequirementCheck.cs | 2 +- .../PowerShellModulesRequirementCheck.cs | 5 +- .../Commands/BlueprintSubcommandTests.cs | 56 ----------- .../Commands/RequirementsSubcommandTests.cs | 14 ++- .../ClientAppRequirementCheckTests.cs | 1 - .../FrontierPreviewRequirementCheckTests.cs | 22 ++--- 15 files changed, 83 insertions(+), 255 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupCommand.cs index 8fd6c909..064fc007 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupCommand.cs @@ -45,7 +45,7 @@ public static Command CreateCommand( // Add subcommands command.AddCommand(RequirementsSubcommand.CreateCommand( - logger, configService, clientAppValidator)); + logger, configService, authValidator, clientAppValidator)); command.AddCommand(InfrastructureSubcommand.CreateCommand( logger, configService, prerequisiteRunner, authValidator, webAppCreator, platformDetector, executor)); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs index 21cc8623..3de16b5d 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs @@ -79,7 +79,7 @@ public static Command CreateCommand( { // Generate correlation ID at workflow entry point var correlationId = HttpClientFactory.GenerateCorrelationId(); - logger.LogInformation("Starting setup all (CorrelationId: {CorrelationId})", correlationId); + logger.LogDebug("Starting setup all (CorrelationId: {CorrelationId})", correlationId); if (dryRun) { @@ -113,61 +113,12 @@ public static Command CreateCommand( return; } - logger.LogInformation("Agent 365 Setup"); - logger.LogInformation("Running all setup steps..."); - - if (skipRequirements) - { - logger.LogInformation("NOTE: Skipping requirements validation (--skip-requirements flag used)"); - } - - if (skipInfrastructure) - { - logger.LogInformation("NOTE: Skipping infrastructure creation (--skip-infrastructure flag used)"); - } - - logger.LogInformation(""); var setupResults = new SetupResults(); try { - // PHASE 0a: CHECK SYSTEM REQUIREMENTS before loading config (if not skipped) - // Runs config-independent checks (PowerShell, Frontier Preview) so blockers - // are surfaced before the user is asked to fill in the configuration wizard. - if (!skipRequirements) - { - logger.LogDebug("Validating system prerequisites..."); - - try - { - var systemResult = await RequirementsSubcommand.RunRequirementChecksAsync( - RequirementsSubcommand.GetSystemRequirementChecks(), - new Agent365Config(), - logger, - category: null, - CancellationToken.None); - - if (!systemResult) - { - logger.LogError("Setup cannot proceed due to the failed requirement checks above. Please fix the issues above and then try again."); - ExceptionHandler.ExitWithCleanup(1); - } - } - catch (Exception reqEx) - { - logger.LogError(reqEx, "Requirements check failed with an unexpected error: {Message}", reqEx.Message); - logger.LogError("Setup cannot proceed because system requirement validation failed unexpectedly."); - logger.LogError("If you want to bypass requirement validation, rerun this command with the --skip-requirements flag."); - ExceptionHandler.ExitWithCleanup(1); - } - } - else - { - logger.LogDebug("Skipping requirements validation (--skip-requirements flag used)"); - } - - // PHASE 0b: Load configuration (may trigger interactive wizard) + // Load configuration var setupConfig = await configService.LoadAsync(config.FullName); // Configure GraphApiService with custom client app ID if available @@ -177,21 +128,19 @@ public static Command CreateCommand( graphApiService.CustomClientAppId = setupConfig.ClientAppId; } - // PHASE 0c: CHECK CONFIG-DEPENDENT REQUIREMENTS after the wizard + // Validate all prerequisites in one pass if (!skipRequirements) { - logger.LogDebug("Validating configuration prerequisites..."); + var checks = RequirementsSubcommand.GetRequirementChecks(authValidator, clientAppValidator); + if (!skipInfrastructure && setupConfig.NeedDeployment) + checks.Add(new InfrastructureRequirementCheck()); try { - var configResult = await RequirementsSubcommand.RunRequirementChecksAsync( - RequirementsSubcommand.GetConfigRequirementChecks(clientAppValidator), - setupConfig, - logger, - category: null, - CancellationToken.None); - - if (!configResult) + var requirementsResult = await RequirementsSubcommand.RunRequirementChecksAsync( + checks, setupConfig, logger, category: null, CancellationToken.None); + + if (!requirementsResult) { logger.LogError("Setup cannot proceed due to the failed requirement checks above. Please fix the issues above and then try again."); ExceptionHandler.ExitWithCleanup(1); @@ -200,36 +149,23 @@ public static Command CreateCommand( catch (Exception reqEx) { logger.LogError(reqEx, "Requirements check failed with an unexpected error: {Message}", reqEx.Message); - logger.LogError("Setup cannot proceed because configuration requirement validation failed unexpectedly."); logger.LogError("If you want to bypass requirement validation, rerun this command with the --skip-requirements flag."); ExceptionHandler.ExitWithCleanup(1); } } - // PHASE 1: VALIDATE ALL PREREQUISITES UPFRONT - logger.LogDebug("Validating all prerequisites..."); - - var prereqChecks = new List - { - new AzureAuthRequirementCheck(authValidator), - new ClientAppRequirementCheck(clientAppValidator) - }; - - if (!skipInfrastructure && setupConfig.NeedDeployment) - prereqChecks.Add(new InfrastructureRequirementCheck()); - // Run advisory environment check (warning only, never blocks) await environmentValidator.ValidateEnvironmentAsync(); - if (!await prerequisiteRunner.RunAsync(prereqChecks, setupConfig, logger, CancellationToken.None)) - { - logger.LogError("Setup cannot proceed due to prerequisite validation failures. Please fix the errors above and try again."); - setupResults.Errors.Add("Prerequisite validation failed"); - ExceptionHandler.ExitWithCleanup(1); - } - logger.LogDebug("All validations passed. Starting setup execution..."); + logger.LogInformation("Running all setup steps... (TraceId: {TraceId})", correlationId); + if (skipRequirements) + logger.LogInformation("NOTE: Requirements validation skipped (--skip-requirements flag used)"); + if (skipInfrastructure) + logger.LogInformation("NOTE: Infrastructure creation skipped (--skip-infrastructure flag used)"); + logger.LogInformation(""); + var generatedConfigPath = Path.Combine( config.DirectoryName ?? Environment.CurrentDirectory, "a365.generated.config.json"); @@ -272,7 +208,6 @@ public static Command CreateCommand( setupConfig, config, executor, - prerequisiteRunner, authValidator, logger, skipInfrastructure, diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs index 5dbc1981..95f83242 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs @@ -124,7 +124,7 @@ public static Command CreateCommand( { // Generate correlation ID at workflow entry point var correlationId = HttpClientFactory.GenerateCorrelationId(); - logger.LogInformation("Starting blueprint setup (CorrelationId: {CorrelationId})", correlationId); + logger.LogDebug("Starting blueprint setup (CorrelationId: {CorrelationId})", correlationId); // Validate mutually exclusive options if (!ValidateMutuallyExclusiveOptions( @@ -184,7 +184,7 @@ await UpdateEndpointAsync( try { var requirementsResult = await RequirementsSubcommand.RunRequirementChecksAsync( - RequirementsSubcommand.GetRequirementChecks(clientAppValidator), + RequirementsSubcommand.GetRequirementChecks(authValidator, clientAppValidator), setupConfig, logger, category: null, @@ -219,6 +219,8 @@ await UpdateEndpointAsync( return; } + logger.LogInformation("Starting blueprint setup... (TraceId: {TraceId})", correlationId); + // Handle --endpoint-only flag if (endpointOnly) { @@ -256,7 +258,6 @@ await CreateBlueprintImplementationAsync( setupConfig, config, executor, - prerequisiteRunner, authValidator, logger, false, @@ -321,7 +322,6 @@ public static async Task CreateBlueprintImplementationA Models.Agent365Config setupConfig, FileInfo config, CommandExecutor executor, - IPrerequisiteRunner prerequisiteRunner, AzureAuthValidator authValidator, ILogger logger, bool skipInfrastructure, @@ -355,21 +355,6 @@ public static async Task CreateBlueprintImplementationA logger.LogInformation(""); logger.LogInformation("==> Creating Agent Blueprint"); - // Validate Azure authentication - var authChecks = new List - { - new AzureAuthRequirementCheck(authValidator) - }; - if (!await prerequisiteRunner.RunAsync(authChecks, setupConfig, logger, cancellationToken)) - { - return new BlueprintCreationResult - { - BlueprintCreated = false, - EndpointRegistered = false, - EndpointRegistrationAttempted = false - }; - } - var generatedConfigPath = Path.Combine( config.DirectoryName ?? Environment.CurrentDirectory, "a365.generated.config.json"); @@ -1143,7 +1128,8 @@ public static async Task EnsureDelegatedConsentWithRetriesAsync( tenantId, objectId, userObjectId: null, - ct); + ct, + scopes: AuthenticationConstants.RequiredClientAppPermissions); if (isOwner) { diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/RequirementsSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/RequirementsSubcommand.cs index 104f5187..f1104679 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/RequirementsSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/RequirementsSubcommand.cs @@ -19,6 +19,7 @@ internal static class RequirementsSubcommand public static Command CreateCommand( ILogger logger, IConfigService configService, + AzureAuthValidator authValidator, IClientAppValidator clientAppValidator) { var command = new Command("requirements", @@ -57,7 +58,7 @@ public static Command CreateCommand( { // Load configuration var setupConfig = await configService.LoadAsync(config.FullName); - var requirementChecks = GetRequirementChecks(clientAppValidator); + var requirementChecks = GetRequirementChecks(authValidator, clientAppValidator); await RunRequirementChecksAsync(requirementChecks, setupConfig, logger, category); } catch (Exception ex) @@ -101,14 +102,13 @@ public static async Task RunRequirementChecksAsync( var warningChecks = 0; var failedChecks = 0; + logger.LogInformation("Checking requirements..."); + // Execute all checks (grouped by category but headers not shown) foreach (var categoryGroup in checksByCategory) { foreach (var check in categoryGroup) { - // Add spacing before each check for readability - Console.WriteLine(); - var result = await check.CheckAsync(setupConfig, logger, ct); if (result.Passed) @@ -129,29 +129,9 @@ public static async Task RunRequirementChecksAsync( } } - // Display summary - logger.LogInformation("Requirements Check Summary"); - logger.LogInformation(new string('=', 50)); - logger.LogInformation("Total checks: {Total}", totalChecks); - logger.LogInformation("Passed: {Passed}", passedChecks); - logger.LogInformation("Warning: {Warning}", warningChecks); - logger.LogInformation("Failed: {Failed}", failedChecks); Console.WriteLine(); - - if (failedChecks > 0) - { - logger.LogError("Some requirements failed. Please address the issues above before running setup."); - logger.LogInformation("Use the resolution guidance provided for each failed check."); - } - else if (warningChecks > 0) - { - logger.LogWarning("All automated checks passed, but {WarningCount} requirement(s) require manual verification.", warningChecks); - logger.LogInformation("Please review the warnings above and ensure all requirements are met before running setup."); - } - else - { - logger.LogInformation("All requirements passed! You're ready to run Agent 365 setup."); - } + logger.LogInformation("Requirements: {Passed} passed, {Warning} warnings, {Failed} failed", + passedChecks, warningChecks, failedChecks); return failedChecks == 0; } @@ -160,10 +140,10 @@ public static async Task RunRequirementChecksAsync( /// Gets all available requirement checks. /// Derived from the union of system and config checks to keep a single source of truth. /// - public static List GetRequirementChecks(IClientAppValidator clientAppValidator) + public static List GetRequirementChecks(AzureAuthValidator authValidator, IClientAppValidator clientAppValidator) { return GetSystemRequirementChecks() - .Concat(GetConfigRequirementChecks(clientAppValidator)) + .Concat(GetConfigRequirementChecks(authValidator, clientAppValidator)) .ToList(); } @@ -186,10 +166,13 @@ public static List GetSystemRequirementChecks() /// /// Gets configuration-dependent requirement checks that must run after the configuration is loaded. /// - public static List GetConfigRequirementChecks(IClientAppValidator clientAppValidator) + public static List GetConfigRequirementChecks(AzureAuthValidator authValidator, IClientAppValidator clientAppValidator) { return new List { + // Azure CLI authentication — required before any Azure operation + new AzureAuthRequirementCheck(authValidator), + // Location configuration — required for endpoint registration new LocationRequirementCheck(), diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/PlatformDetector.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/PlatformDetector.cs index 3ce648c5..38473bc2 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/PlatformDetector.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/PlatformDetector.cs @@ -29,7 +29,7 @@ public Models.ProjectPlatform Detect(string projectPath) return Models.ProjectPlatform.Unknown; } - _logger.LogInformation("Detecting platform in: {Path}", projectPath); + _logger.LogDebug("Detecting platform in: {Path}", projectPath); // Check for .NET project files var dotnetFiles = Directory.GetFiles(projectPath, "*.csproj", SearchOption.TopDirectoryOnly) @@ -39,7 +39,7 @@ public Models.ProjectPlatform Detect(string projectPath) if (dotnetFiles.Length > 0) { - _logger.LogInformation("Detected .NET project (found {Count} project file(s))", dotnetFiles.Length); + _logger.LogDebug("Detected .NET project (found {Count} project file(s))", dotnetFiles.Length); return Models.ProjectPlatform.DotNet; } @@ -50,7 +50,7 @@ public Models.ProjectPlatform Detect(string projectPath) if (File.Exists(packageJsonPath) || jsFiles || tsFiles) { - _logger.LogInformation("Detected Node.js project"); + _logger.LogDebug("Detected Node.js project"); return Models.ProjectPlatform.NodeJs; } @@ -62,7 +62,7 @@ public Models.ProjectPlatform Detect(string projectPath) if (File.Exists(requirementsPath) || File.Exists(setupPyPath) || File.Exists(pyprojectPath) || pythonFiles.Length > 0) { - _logger.LogInformation("Detected Python project"); + _logger.LogDebug("Detected Python project"); return Models.ProjectPlatform.Python; } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementCheck.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementCheck.cs index f3cacf21..d0292f9f 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementCheck.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementCheck.cs @@ -23,24 +23,13 @@ public abstract class RequirementCheck : IRequirementCheck /// public abstract Task CheckAsync(Agent365Config config, ILogger logger, CancellationToken cancellationToken = default); - /// - /// Helper method to log check start - /// - protected virtual void LogCheckStart(ILogger logger) - { - logger.LogInformation("Requirement: {Name}", Name); - } - /// /// Helper method to log check success /// protected virtual void LogCheckSuccess(ILogger logger, string? details = null) { - logger.LogInformation("[PASS] {Name}: PASSED", Name); - if (!string.IsNullOrWhiteSpace(details)) - { - logger.LogInformation(" Details: {Details}", details); - } + logger.LogInformation("[PASS] {Name}{Details}", Name, + string.IsNullOrWhiteSpace(details) ? "" : $" ({details})"); } /// @@ -48,11 +37,8 @@ protected virtual void LogCheckSuccess(ILogger logger, string? details = null) /// protected virtual void LogCheckWarning(ILogger logger, string? details = null) { - logger.LogWarning("[WARNING] {Name}: Cannot automatically verify", Name); - if (!string.IsNullOrWhiteSpace(details)) - { - logger.LogWarning(" Details: {Details}", details); - } + logger.LogInformation("[WARN] {Name}{Details}", Name, + string.IsNullOrWhiteSpace(details) ? "" : $" - {details}"); } /// @@ -60,9 +46,9 @@ protected virtual void LogCheckWarning(ILogger logger, string? details = null) /// protected virtual void LogCheckFailure(ILogger logger, string errorMessage, string resolutionGuidance) { - logger.LogError("[FAIL] {Name}: FAILED", Name); - logger.LogError(" Issue: {ErrorMessage}", errorMessage); - logger.LogError(" Resolution: {ResolutionGuidance}", resolutionGuidance); + logger.LogInformation("[FAIL] {Name}", Name); + logger.LogInformation(" Issue: {ErrorMessage}", errorMessage); + logger.LogInformation(" Resolution: {ResolutionGuidance}", resolutionGuidance); } /// @@ -74,7 +60,6 @@ protected async Task ExecuteCheckWithLoggingAsync( Func> checkImplementation, CancellationToken cancellationToken = default) { - LogCheckStart(logger); try { diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/AzureAuthRequirementCheck.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/AzureAuthRequirementCheck.cs index 3fe62dc2..ef0e7c96 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/AzureAuthRequirementCheck.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/AzureAuthRequirementCheck.cs @@ -34,8 +34,14 @@ public override async Task CheckAsync( ILogger logger, CancellationToken cancellationToken = default) { - // AzureAuthValidator logs all detailed user-facing messages internally. - // This adapter converts the bool result into a RequirementCheckResult. + return await ExecuteCheckWithLoggingAsync(config, logger, CheckImplementationAsync, cancellationToken); + } + + private async Task CheckImplementationAsync( + Agent365Config config, + ILogger logger, + CancellationToken cancellationToken) + { var authenticated = await _authValidator.ValidateAuthenticationAsync(config.SubscriptionId); if (!authenticated) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/ClientAppRequirementCheck.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/ClientAppRequirementCheck.cs index a90caddb..7d485788 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/ClientAppRequirementCheck.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/ClientAppRequirementCheck.cs @@ -70,7 +70,7 @@ await _clientAppValidator.EnsureValidClientAppAsync( ); return RequirementCheckResult.Success( - details: $"Client app {config.ClientAppId} is properly configured with all required permissions and admin consent." + details: config.ClientAppId ); } catch (ClientAppValidationException ex) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/FrontierPreviewRequirementCheck.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/FrontierPreviewRequirementCheck.cs index a887f1e3..df1a2258 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/FrontierPreviewRequirementCheck.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/FrontierPreviewRequirementCheck.cs @@ -24,21 +24,10 @@ public class FrontierPreviewRequirementCheck : RequirementCheck /// public override Task CheckAsync(Agent365Config config, ILogger logger, CancellationToken cancellationToken = default) { - logger.LogInformation("Requirement: {Name}", Name); - - Console.WriteLine(); - logger.LogWarning("While Microsoft Agent 365 is in preview, Frontier Preview Program enrollment is required."); - Console.WriteLine(" - Enrollment cannot be verified automatically."); - Console.WriteLine(" - Please confirm your tenant is enrolled before continuing."); - Console.WriteLine(); - Console.WriteLine("Documentation:"); - Console.WriteLine(" - https://learn.microsoft.com/microsoft-agent-365/developer/"); - Console.WriteLine(" - https://adoption.microsoft.com/copilot/frontier-program/"); - - // Return warning without using base class logging (already logged above) - return Task.FromResult(RequirementCheckResult.Warning( - message: "Cannot automatically verify Frontier Preview Program enrollment", - details: "Tenant must be enrolled in Frontier Preview Program during Agent 365 preview. Check documentation to verify if this requirement still applies." - )); + return ExecuteCheckWithLoggingAsync(config, logger, (_, __, ___) => Task.FromResult( + RequirementCheckResult.Warning( + message: "Cannot automatically verify Frontier Preview Program enrollment", + details: "enrollment cannot be auto-verified. See: https://adoption.microsoft.com/copilot/frontier-program/" + )), cancellationToken); } } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/LocationRequirementCheck.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/LocationRequirementCheck.cs index 14f73ae6..e810d2c6 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/LocationRequirementCheck.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/LocationRequirementCheck.cs @@ -40,7 +40,7 @@ private static Task CheckImplementationAsync(Agent365Con } return Task.FromResult(RequirementCheckResult.Success( - details: $"Location is configured: {config.Location}" + details: config.Location?.Trim() )); } } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/PowerShellModulesRequirementCheck.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/PowerShellModulesRequirementCheck.cs index 85871072..fb2687be 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/PowerShellModulesRequirementCheck.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/PowerShellModulesRequirementCheck.cs @@ -43,8 +43,6 @@ public override async Task CheckAsync(Agent365Config con /// private async Task CheckImplementationAsync(Agent365Config config, ILogger logger, CancellationToken cancellationToken) { - logger.LogInformation("Checking if PowerShell is available on this system..."); - // Check if PowerShell is available var powerShellAvailable = await CheckPowerShellAvailabilityAsync(logger, cancellationToken); if (!powerShellAvailable) @@ -65,7 +63,6 @@ private async Task CheckImplementationAsync(Agent365Conf ); } - logger.LogInformation("Checking PowerShell modules..."); var missingModules = new List(); var installedModules = new List(); @@ -91,7 +88,7 @@ private async Task CheckImplementationAsync(Agent365Conf if (missingModules.Count == 0) { return RequirementCheckResult.Success( - details: $"All required PowerShell modules are installed: {string.Join(", ", installedModules.Select(m => m.Name))}" + details: string.Join(", ", installedModules.Select(m => m.Name)) ); } diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BlueprintSubcommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BlueprintSubcommandTests.cs index ad36bc92..445dba95 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BlueprintSubcommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BlueprintSubcommandTests.cs @@ -290,7 +290,6 @@ public async Task CreateBlueprintImplementation_WithMissingDisplayName_ShouldThr config, configFile, _mockExecutor, - _mockPrerequisiteRunner, _mockAuthValidator, _mockLogger, skipInfrastructure: false, @@ -306,53 +305,6 @@ public async Task CreateBlueprintImplementation_WithMissingDisplayName_ShouldThr result.EndpointRegistered.Should().BeFalse(); } - [Fact] - public async Task CreateBlueprintImplementation_WithAzureValidationFailure_ShouldReturnFalse() - { - // Arrange - var config = new Agent365Config - { - TenantId = "00000000-0000-0000-0000-000000000000", - ClientAppId = "a1b2c3d4-e5f6-a7b8-c9d0-e1f2a3b4c5d6", // Required for validation - SubscriptionId = "test-sub", - AgentBlueprintDisplayName = "Test Blueprint", - Location = "eastus" // Required for endpoint registration; location guard runs before Azure validation - }; - - var configFile = new FileInfo("test-config.json"); - - _mockPrerequisiteRunner.RunAsync( - Arg.Any>(), - Arg.Any(), - Arg.Any(), - Arg.Any()) - .Returns(false); // Auth check fails - - // Act - var result = await BlueprintSubcommand.CreateBlueprintImplementationAsync( - config, - configFile, - _mockExecutor, - _mockPrerequisiteRunner, - _mockAuthValidator, - _mockLogger, - skipInfrastructure: false, - isSetupAll: false, - _mockConfigService, - _mockBotConfigurator, - _mockPlatformDetector, - _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService); - - // Assert - result.Should().NotBeNull(); - result.BlueprintCreated.Should().BeFalse(); - result.EndpointRegistered.Should().BeFalse(); - await _mockPrerequisiteRunner.Received(1).RunAsync( - Arg.Any>(), - Arg.Any(), - Arg.Any(), - Arg.Any()); - } [Fact] public void CommandDescription_ShouldMentionRequiredPermissions() @@ -535,19 +487,11 @@ public async Task CreateBlueprintImplementation_ShouldLogProgressMessages() var configFile = new FileInfo("test-config.json"); - _mockPrerequisiteRunner.RunAsync( - Arg.Any>(), - Arg.Any(), - Arg.Any(), - Arg.Any()) - .Returns(false); // Fail fast for this test - // Act var result = await BlueprintSubcommand.CreateBlueprintImplementationAsync( config, configFile, _mockExecutor, - _mockPrerequisiteRunner, _mockAuthValidator, _mockLogger, skipInfrastructure: false, diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/RequirementsSubcommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/RequirementsSubcommandTests.cs index 9e02d09f..08afe944 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/RequirementsSubcommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/RequirementsSubcommandTests.cs @@ -9,6 +9,7 @@ using Microsoft.Agents.A365.DevTools.Cli.Services.Requirements.RequirementChecks; using Microsoft.Agents.A365.DevTools.Cli.Tests.TestHelpers; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using NSubstitute; using Xunit; @@ -161,13 +162,16 @@ public void GetRequirementChecks_ContainsAllExpectedCheckTypes() { // GetRequirementChecks is now derived from GetSystemRequirementChecks + GetConfigRequirementChecks. // This test guards against a check being accidentally added to one sub-list but not propagated. + var mockExecutor = Substitute.ForPartsOf(Substitute.For>()); + var mockAuthValidator = Substitute.ForPartsOf(NullLogger.Instance, mockExecutor); var mockValidator = Substitute.For(); - var checks = RequirementsSubcommand.GetRequirementChecks(mockValidator); + var checks = RequirementsSubcommand.GetRequirementChecks(mockAuthValidator, mockValidator); - checks.Should().HaveCount(4, "system (2) + config (2) checks"); + checks.Should().HaveCount(5, "system (2) + config (3) checks"); checks.Should().ContainSingle(c => c is FrontierPreviewRequirementCheck); checks.Should().ContainSingle(c => c is PowerShellModulesRequirementCheck); + checks.Should().ContainSingle(c => c is AzureAuthRequirementCheck); checks.Should().ContainSingle(c => c is LocationRequirementCheck); checks.Should().ContainSingle(c => c is ClientAppRequirementCheck); } @@ -176,11 +180,13 @@ public void GetRequirementChecks_ContainsAllExpectedCheckTypes() public void GetRequirementChecks_IsUnionOfSystemAndConfigChecks() { // GetRequirementChecks must exactly equal GetSystemRequirementChecks + GetConfigRequirementChecks. + var mockExecutor = Substitute.ForPartsOf(Substitute.For>()); + var mockAuthValidator = Substitute.ForPartsOf(NullLogger.Instance, mockExecutor); var mockValidator = Substitute.For(); - var all = RequirementsSubcommand.GetRequirementChecks(mockValidator); + var all = RequirementsSubcommand.GetRequirementChecks(mockAuthValidator, mockValidator); var system = RequirementsSubcommand.GetSystemRequirementChecks(); - var config = RequirementsSubcommand.GetConfigRequirementChecks(mockValidator); + var config = RequirementsSubcommand.GetConfigRequirementChecks(mockAuthValidator, mockValidator); all.Should().HaveCount(system.Count + config.Count); all.Select(c => c.GetType()).Should().StartWith(system.Select(c => c.GetType()), diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Requirements/ClientAppRequirementCheckTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Requirements/ClientAppRequirementCheckTests.cs index f90ecc32..fa2ba32f 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Requirements/ClientAppRequirementCheckTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Requirements/ClientAppRequirementCheckTests.cs @@ -94,7 +94,6 @@ public async Task CheckAsync_WithValidClientApp_ShouldReturnSuccess() // Assert result.Should().NotBeNull(); result.Passed.Should().BeTrue("client app is valid"); - result.Details.Should().Contain("properly configured"); result.Details.Should().Contain(config.ClientAppId); result.ErrorMessage.Should().BeNullOrEmpty(); result.ResolutionGuidance.Should().BeNullOrEmpty(); diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Requirements/FrontierPreviewRequirementCheckTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Requirements/FrontierPreviewRequirementCheckTests.cs index 41d651da..6c83b7b5 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Requirements/FrontierPreviewRequirementCheckTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Requirements/FrontierPreviewRequirementCheckTests.cs @@ -37,8 +37,7 @@ public async Task CheckAsync_ShouldReturnWarning_WithDetails() result.Passed.Should().BeTrue("check should pass to allow user to proceed despite warning"); result.IsWarning.Should().BeTrue("check should be flagged as a warning"); result.Details.Should().NotBeNullOrEmpty(); - result.Details.Should().Contain("enrolled"); - result.Details.Should().Contain("preview"); + result.Details.Should().Contain("auto-verified"); result.ErrorMessage.Should().Contain("Cannot automatically verify"); result.ResolutionGuidance.Should().BeNullOrEmpty("warning checks don't have resolution guidance"); } @@ -54,11 +53,11 @@ public async Task CheckAsync_ShouldLogMainWarningMessage() await check.CheckAsync(config, _mockLogger); // Assert - // Verify the logger was called with the main warning message + // Verify the logger was called with the warning output line _mockLogger.Received().Log( - LogLevel.Warning, + LogLevel.Information, Arg.Any(), - Arg.Is(o => o.ToString()!.Contains("Frontier Preview Program enrollment is required")), + Arg.Is(o => o.ToString()!.Contains("[WARN] Frontier Preview Program")), Arg.Any(), Arg.Any>()); } @@ -74,11 +73,11 @@ public async Task CheckAsync_ShouldLogRequirementName() await check.CheckAsync(config, _mockLogger); // Assert - // Verify the logger was called with "Requirement:" prefix + // Verify the logger was called with the check name in the output line _mockLogger.Received().Log( LogLevel.Information, Arg.Any(), - Arg.Is(o => o.ToString()!.Contains("Requirement:") && o.ToString()!.Contains("Frontier Preview Program")), + Arg.Is(o => o.ToString()!.Contains("Frontier Preview Program")), Arg.Any(), Arg.Any>()); } @@ -94,9 +93,8 @@ public async Task CheckAsync_ShouldIncludePreviewContext() var result = await check.CheckAsync(config, _mockLogger); // Assert - // Verify the result mentions preview context - result.Details.Should().Contain("preview"); - result.Details.Should().Contain("enrolled"); + // Verify the result mentions the auto-verification limitation + result.Details.Should().Contain("auto-verified"); } [Fact] @@ -110,8 +108,8 @@ public async Task CheckAsync_ShouldMentionDocumentationCheck() var result = await check.CheckAsync(config, _mockLogger); // Assert - // Verify the details mention checking documentation - result.Details.Should().Contain("Check documentation"); + // Verify the details include a reference URL + result.Details.Should().Contain("https://adoption.microsoft.com/copilot/frontier-program/"); } [Fact]