diff --git a/CHANGELOG.md b/CHANGELOG.md index e84f1fe2..64adb4b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] +### Added +- `a365 cleanup azure --dry-run` — preview resources that would be deleted without making any changes or requiring Azure authentication +- `AppServiceAuthRequirementCheck` — validates App Service deployment token before `a365 deploy` begins, catching revoked grants (AADSTS50173) early +- `MosPrerequisitesRequirementCheck` — validates MOS service principals before `a365 publish` proceeds + ### Fixed - macOS/Linux: device code fallback when browser authentication is unavailable (#309) - Linux: MSAL fallback when PowerShell `Connect-MgGraph` fails in non-TTY environments (#309) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs index 6def19ba..d255d659 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs @@ -6,9 +6,12 @@ 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; using Microsoft.Agents.A365.DevTools.Cli.Models; +using Microsoft.Agents.A365.DevTools.Cli.Commands.SetupSubcommands; namespace Microsoft.Agents.A365.DevTools.Cli.Commands; @@ -17,6 +20,13 @@ public class CleanupCommand private const string AgenticUsersKey = "agentic users"; private const string IdentitySpsKey = "identity SPs"; + /// + /// Returns the base requirement checks for cleanup operations: + /// Azure authentication only. + /// + public static List GetBaseChecks(AzureAuthValidator auth) + => [new AzureAuthRequirementCheck(auth)]; + public static Command CreateCommand( ILogger logger, IConfigService configService, @@ -24,7 +34,8 @@ public static Command CreateCommand( CommandExecutor executor, AgentBlueprintService agentBlueprintService, IConfirmationProvider confirmationProvider, - FederatedCredentialService federatedCredentialService) + FederatedCredentialService federatedCredentialService, + AzureAuthValidator authValidator) { var cleanupCommand = new Command("cleanup", "Clean up ALL resources (blueprint, instance, Azure) - use subcommands for granular cleanup"); @@ -55,7 +66,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, authValidator)); cleanupCommand.AddCommand(CreateInstanceCleanupCommand(logger, configService, executor)); return cleanupCommand; @@ -301,13 +312,20 @@ private static Command CreateBlueprintCleanupCommand( return command; } + /// + /// Returns the requirement checks for the cleanup azure subcommand. + /// + internal static List GetAzureCleanupChecks(AzureAuthValidator auth) + => GetBaseChecks(auth); + private static Command CreateAzureCleanupCommand( ILogger logger, IConfigService configService, - CommandExecutor executor) + CommandExecutor executor, + AzureAuthValidator authValidator) { var command = new Command("azure", "Remove Azure resources (App Service, App Service Plan)"); - + var configOption = new Option( new[] { "--config", "-c" }, "Path to configuration file") @@ -319,18 +337,28 @@ private static Command CreateAzureCleanupCommand( new[] { "--verbose", "-v" }, description: "Enable verbose logging"); + var dryRunOption = new Option("--dry-run", "Show resources that would be deleted without making any changes"); + command.AddOption(configOption); command.AddOption(verboseOption); + command.AddOption(dryRunOption); - command.SetHandler(async (configFile, verbose) => + command.SetHandler(async (configFile, verbose, dryRun) => { try { - logger.LogInformation("Starting Azure cleanup..."); - + if (!dryRun) + logger.LogInformation("Starting Azure cleanup..."); + var config = await LoadConfigAsync(configFile, logger, configService); if (config == null) return; + if (!dryRun) + { + var checks = GetAzureCleanupChecks(authValidator); + await RequirementsSubcommand.RunChecksOrExitAsync(checks, config, logger, CancellationToken.None); + } + logger.LogInformation(""); logger.LogInformation("Azure Cleanup Preview:"); logger.LogInformation("========================="); @@ -341,6 +369,12 @@ private static Command CreateAzureCleanupCommand( logger.LogInformation(" Resource Group: {ResourceGroup}", config.ResourceGroup); logger.LogInformation(""); + if (dryRun) + { + logger.LogInformation("DRY RUN: No changes made."); + return; + } + Console.Write("Continue with Azure cleanup? (y/N): "); var response = Console.ReadLine()?.Trim().ToLowerInvariant(); if (response != "y" && response != "yes") @@ -397,7 +431,7 @@ private static Command CreateAzureCleanupCommand( { logger.LogError(ex, "Azure cleanup failed with exception"); } - }, configOption, verboseOption); + }, configOption, verboseOption, dryRunOption); return command; } @@ -962,9 +996,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("{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 bb03a1e0..1ec70dc5 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; @@ -18,7 +19,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 +76,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 @@ -505,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("{Message}", ex.IssueDescription); return null; } catch (Exception ex) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DeployCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DeployCommand.cs index 62cf2a83..244c535b 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DeployCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DeployCommand.cs @@ -1,12 +1,15 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using Microsoft.Agents.A365.DevTools.Cli.Commands.SetupSubcommands; using Microsoft.Agents.A365.DevTools.Cli.Constants; using Microsoft.Agents.A365.DevTools.Cli.Exceptions; 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.Helpers; +using Microsoft.Agents.A365.DevTools.Cli.Services.Requirements; +using Microsoft.Agents.A365.DevTools.Cli.Services.Requirements.RequirementChecks; using Microsoft.Extensions.Logging; using System.CommandLine; @@ -19,7 +22,7 @@ public static Command CreateCommand( IConfigService configService, CommandExecutor executor, DeploymentService deploymentService, - IAzureValidator azureValidator, + AzureAuthValidator authValidator, GraphApiService graphApiService, AgentBlueprintService blueprintService) { @@ -53,7 +56,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, authValidator)); command.AddCommand(CreateMcpSubcommand(logger, configService, executor, graphApiService, blueprintService)); // Single handler for the deploy command - runs only the application deployment flow @@ -82,7 +85,7 @@ public static Command CreateCommand( } var validatedConfig = await ValidateDeploymentPrerequisitesAsync( - config.FullName, configService, azureValidator, executor, logger); + config.FullName, configService, authValidator, executor, logger); if (validatedConfig == null) return; await DeployApplicationAsync(validatedConfig, deploymentService, verbose, inspect, restart, logger); @@ -96,12 +99,20 @@ public static Command CreateCommand( return command; } + /// + /// Requirement checks for deploy: Azure CLI auth and App Service token validity. + /// AppServiceAuthRequirementCheck probes the App Service scope explicitly to catch + /// revoked/expired grants before build and upload begin. + /// + public static List GetChecks(AzureAuthValidator auth) + => [new AzureAuthRequirementCheck(auth), new AppServiceAuthRequirementCheck(auth)]; + private static Command CreateAppSubcommand( ILogger logger, IConfigService configService, CommandExecutor executor, DeploymentService deploymentService, - IAzureValidator azureValidator) + AzureAuthValidator authValidator) { var command = new Command("app", "Deploy Microsoft Agent 365 application binaries to the configured Azure App Service"); @@ -157,7 +168,7 @@ private static Command CreateAppSubcommand( } var validatedConfig = await ValidateDeploymentPrerequisitesAsync( - config.FullName, configService, azureValidator, executor, logger); + config.FullName, configService, authValidator, executor, logger); if (validatedConfig == null) return; await DeployApplicationAsync(validatedConfig, deploymentService, verbose, inspect, restart, logger); @@ -218,6 +229,23 @@ 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."); + ExceptionHandler.ExitWithCleanup(1); + } + if (string.IsNullOrWhiteSpace(updateConfig.AgenticAppId)) + { + logger.LogError("agenticAppId is not configured. Run 'a365 setup all' to complete setup."); + ExceptionHandler.ExitWithCleanup(1); + } + if (string.IsNullOrWhiteSpace(updateConfig.TenantId)) + { + logger.LogError("tenantId is not configured. Run 'a365 setup all' to complete setup."); + ExceptionHandler.ExitWithCleanup(1); + } + // Configure GraphApiService with custom client app ID if available if (!string.IsNullOrWhiteSpace(updateConfig.ClientAppId)) { @@ -241,26 +269,40 @@ private static Command CreateMcpSubcommand( } /// - /// Validates configuration, Azure authentication, and Web App existence + /// Validates configuration, Azure CLI authentication, and Web App existence /// private static async Task ValidateDeploymentPrerequisitesAsync( string configPath, IConfigService configService, - IAzureValidator azureValidator, + AzureAuthValidator authValidator, CommandExecutor executor, ILogger logger) { // Load configuration var configData = await configService.LoadAsync(configPath); - if (configData == null) return null; + if (configData == null) + { + Environment.ExitCode = 1; + return null; + } - // Validate Azure CLI authentication, subscription, and environment - if (!await azureValidator.ValidateAllAsync(configData.SubscriptionId)) + // Validate required config fields before any network calls + var missingFields = new List(); + if (string.IsNullOrWhiteSpace(configData.ResourceGroup)) missingFields.Add("resourceGroup"); + if (string.IsNullOrWhiteSpace(configData.WebAppName)) missingFields.Add("webAppName"); + if (string.IsNullOrWhiteSpace(configData.SubscriptionId)) missingFields.Add("subscriptionId"); + if (missingFields.Count > 0) { - logger.LogError("Deployment cannot proceed without proper Azure CLI authentication and the correct subscription context"); + logger.LogError("Missing required configuration fields: {Fields}. Update a365.config.json and retry.", + string.Join(", ", missingFields)); + Environment.ExitCode = 1; return null; } + // Validate Azure CLI authentication and App Service token scope + await RequirementsSubcommand.RunChecksOrExitAsync( + GetChecks(authValidator), configData, logger, CancellationToken.None); + // Validate Azure Web App exists before starting deployment logger.LogInformation("Validating Azure Web App exists..."); var checkResult = await executor.ExecuteAsync("az", @@ -278,6 +320,7 @@ private static Command CreateMcpSubcommand( logger.LogInformation(" 2. Or verify your a365.config.json has the correct WebAppName and ResourceGroup"); logger.LogInformation(""); logger.LogError("Deployment cannot proceed without a valid Azure Web App target"); + Environment.ExitCode = 1; return null; } @@ -482,9 +525,11 @@ private static void HandleDeploymentException(Exception ex, ILogger logger) logger.LogInformation(" 3. Run 'a365 deploy' to perform a deployment"); logger.LogInformation(""); break; + case DeployAppException deployAppEx: + // Already a structured exception with clean message — let it propagate as-is + throw deployAppEx; default: logger.LogError("Deployment failed: {Message}", ex.Message); - throw new DeployAppException($"Deployment failed: {ex.Message}", ex); } } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommand.cs index 20674962..21a3a7c3 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommand.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using Microsoft.Agents.A365.DevTools.Cli.Commands.SetupSubcommands; using Microsoft.Agents.A365.DevTools.Cli.Constants; using Microsoft.Agents.A365.DevTools.Cli.Exceptions; using Microsoft.Agents.A365.DevTools.Cli.Helpers; @@ -8,6 +9,8 @@ using Microsoft.Agents.A365.DevTools.Cli.Services; using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; using Microsoft.Agents.A365.DevTools.Cli.Services.Internal; +using Microsoft.Agents.A365.DevTools.Cli.Services.Requirements; +using Microsoft.Agents.A365.DevTools.Cli.Services.Requirements.RequirementChecks; using Microsoft.Extensions.Logging; using System.CommandLine; using System.IO.Compression; @@ -83,6 +86,13 @@ private static string GetProjectDirectory(Agent365Config config, ILogger logger) } } + /// + /// Requirement checks for publish: MOS service principals must exist and be configured. + /// Runs before the interactive manifest editing pause to avoid wasted work. + /// + internal static List GetChecks(GraphApiService graphApiService, AgentBlueprintService blueprintService) + => [new MosPrerequisitesRequirementCheck(graphApiService, blueprintService)]; + public static Command CreateCommand( ILogger logger, IConfigService configService, @@ -140,6 +150,12 @@ public static Command CreateCommand( return; } + if (!skipGraph && string.IsNullOrWhiteSpace(config.ClientAppId)) + { + logger.LogError("clientAppId is not configured. Run 'a365 setup blueprint' first to configure client app authentication."); + return; + } + // Use deploymentProjectPath from config for portability var baseDir = GetProjectDirectory(config, logger); var manifestDir = Path.Combine(baseDir, "manifest"); @@ -190,10 +206,14 @@ public static Command CreateCommand( var mosTitlesBaseUrl = GetMosTitlesUrl(tenantId); logger.LogInformation("Using MOS Titles URL: {Url} (Tenant: {TenantId})", mosTitlesBaseUrl, tenantId ?? "unknown"); - // Warn if tenantId is missing if (string.IsNullOrWhiteSpace(tenantId)) { - logger.LogWarning("tenantId missing in configuration; using default production MOS URL. Graph operations will be skipped."); + if (!skipGraph) + { + logger.LogError("tenantId is not configured. Graph operations require tenantId. Use --skip-graph to publish without Graph operations, or run 'a365 setup all' to complete setup."); + return; + } + logger.LogWarning("tenantId missing in configuration; using default production MOS URL. Graph operations will be skipped (--skip-graph)."); } string updatedManifest = await UpdateManifestFileAsync(logger, agentBlueprintDisplayName, blueprintId, manifestPath); @@ -217,6 +237,14 @@ public static Command CreateCommand( logger.LogDebug("Manifest files written to disk"); + // Verify MOS prerequisites before asking user to edit manifest (fail fast) + // Skip when --skip-graph is set: MOS checks perform Graph operations + if (!skipGraph) + { + await RequirementsSubcommand.RunChecksOrExitAsync( + GetChecks(graphApiService, blueprintService), config, logger, context.GetCancellationToken()); + } + // Interactive pause for user customization logger.LogInformation(""); logger.LogInformation("=== MANIFEST UPDATED ==="); @@ -314,33 +342,6 @@ public static Command CreateCommand( } logger.LogInformation("Created archive {ZipPath}", zipPath); - // Ensure MOS prerequisites are configured (service principals + permissions) - try - { - logger.LogInformation(""); - logger.LogDebug("Checking MOS prerequisites (service principals and permissions)..."); - var mosPrereqsConfigured = await PublishHelpers.EnsureMosPrerequisitesAsync( - graphApiService, blueprintService, config, logger); - - if (!mosPrereqsConfigured) - { - logger.LogError("Failed to configure MOS prerequisites. Aborting publish."); - return; - } - logger.LogInformation(""); - } - catch (SetupValidationException ex) - { - logger.LogError("MOS prerequisites configuration failed: {Message}", ex.Message); - logger.LogInformation(""); - logger.LogInformation("To manually create MOS service principals, run:"); - logger.LogInformation(" az ad sp create --id 6ec511af-06dc-4fe2-b493-63a37bc397b1"); - logger.LogInformation(" az ad sp create --id 8578e004-a5c6-46e7-913e-12f58912df43"); - logger.LogInformation(" az ad sp create --id e8be65d6-d430-4289-a665-51bf2a194bda"); - logger.LogInformation(""); - return; - } - // Acquire MOS token using native C# service logger.LogDebug("Acquiring MOS authentication token for environment: {Environment}", mosEnv); var cleanLoggerFactory = LoggerFactoryHelper.CreateCleanLoggerFactory(); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupCommand.cs index 7a6aef71..fe83268d 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupCommand.cs @@ -3,6 +3,8 @@ using Microsoft.Agents.A365.DevTools.Cli.Commands.SetupSubcommands; 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.Extensions.Logging; using System.CommandLine; @@ -14,13 +16,24 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Commands /// public class SetupCommand { + /// + /// Returns the base requirement checks shared by all setup subcommands: + /// Azure authentication, Frontier Preview enrollment, and PowerShell modules. + /// + public static List GetBaseChecks(AzureAuthValidator auth) + => [ + new AzureAuthRequirementCheck(auth), + new FrontierPreviewRequirementCheck(), + new PowerShellModulesRequirementCheck() + ]; + public static Command CreateCommand( ILogger logger, IConfigService configService, CommandExecutor executor, DeploymentService deploymentService, IBotConfigurator botConfigurator, - IAzureValidator azureValidator, + AzureAuthValidator authValidator, PlatformDetector platformDetector, GraphApiService graphApiService, AgentBlueprintService blueprintService, @@ -28,7 +41,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" + @@ -42,19 +55,19 @@ public static Command CreateCommand( // Add subcommands command.AddCommand(RequirementsSubcommand.CreateCommand( - logger, configService, clientAppValidator)); + logger, configService, authValidator, clientAppValidator)); command.AddCommand(InfrastructureSubcommand.CreateCommand( - logger, configService, azureValidator, platformDetector, executor)); + logger, configService, authValidator, platformDetector, executor)); command.AddCommand(BlueprintSubcommand.CreateCommand( - logger, configService, executor, azureValidator, platformDetector, botConfigurator, graphApiService, blueprintService, clientAppValidator, blueprintLookupService, federatedCredentialService)); + logger, configService, executor, authValidator, platformDetector, botConfigurator, graphApiService, blueprintService, clientAppValidator, blueprintLookupService, federatedCredentialService)); command.AddCommand(PermissionsSubcommand.CreateCommand( - logger, configService, executor, graphApiService, blueprintService)); + logger, authValidator, configService, executor, graphApiService, blueprintService)); command.AddCommand(AllSubcommand.CreateCommand( - logger, configService, executor, botConfigurator, azureValidator, platformDetector, graphApiService, blueprintService, clientAppValidator, blueprintLookupService, federatedCredentialService)); + logger, configService, executor, botConfigurator, authValidator, 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 39f6a421..152b2de1 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs @@ -1,11 +1,14 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using Microsoft.Agents.A365.DevTools.Cli.Commands; using Microsoft.Agents.A365.DevTools.Cli.Constants; using Microsoft.Agents.A365.DevTools.Cli.Exceptions; 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; +using Microsoft.Agents.A365.DevTools.Cli.Services.Requirements.RequirementChecks; using Microsoft.Extensions.Logging; using System.CommandLine; @@ -21,12 +24,35 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Commands.SetupSubcommands; /// internal static class AllSubcommand { + /// + /// Returns the requirement checks for setup all. + /// Composes SetupCommand base checks + Location + ClientApp + optional Infrastructure. + /// + public static List GetChecks( + AzureAuthValidator auth, + IClientAppValidator clientAppValidator, + bool includeInfrastructure) + { + var checks = new List(SetupCommand.GetBaseChecks(auth)) + { + new LocationRequirementCheck(), + new ClientAppRequirementCheck(clientAppValidator), + }; + + if (includeInfrastructure) + { + checks.Add(new InfrastructureRequirementCheck()); + } + + return checks; + } + public static Command CreateCommand( ILogger logger, IConfigService configService, CommandExecutor executor, IBotConfigurator botConfigurator, - IAzureValidator azureValidator, + AzureAuthValidator authValidator, PlatformDetector platformDetector, GraphApiService graphApiService, AgentBlueprintService blueprintService, @@ -75,7 +101,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) { @@ -109,61 +135,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 @@ -173,94 +150,34 @@ 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 includeInfra = !skipInfrastructure && setupConfig.NeedDeployment; + var checks = AllSubcommand.GetChecks(authValidator, clientAppValidator, includeInfra); try { - var configResult = await RequirementsSubcommand.RunRequirementChecksAsync( - RequirementsSubcommand.GetConfigRequirementChecks(clientAppValidator), - setupConfig, - logger, - category: null, - CancellationToken.None); - - if (!configResult) - { - logger.LogError("Setup cannot proceed due to the failed requirement checks above. Please fix the issues above and then try again."); - ExceptionHandler.ExitWithCleanup(1); - } + await RequirementsSubcommand.RunChecksOrExitAsync( + checks, setupConfig, logger, CancellationToken.None); } - catch (Exception reqEx) + catch (Exception reqEx) when (reqEx is not OperationCanceledException) { 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 allErrors = new List(); - - // Validate Azure CLI authentication first - logger.LogDebug("Validating Azure CLI authentication..."); - if (!await azureValidator.ValidateAllAsync(setupConfig.SubscriptionId)) - { - 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"); - } - - // 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"); - } - } - - // 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"); - } - - // Stop if any validation failed - if (allErrors.Count > 0) - { - 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); - 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"); @@ -303,7 +220,7 @@ public static Command CreateCommand( setupConfig, config, executor, - azureValidator, + 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 c6dc5cb6..6c047629 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs @@ -3,12 +3,15 @@ using Azure.Core; using Azure.Identity; +using Microsoft.Agents.A365.DevTools.Cli.Commands; using Microsoft.Agents.A365.DevTools.Cli.Constants; using Microsoft.Agents.A365.DevTools.Cli.Exceptions; 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.Helpers; +using Microsoft.Agents.A365.DevTools.Cli.Services.Requirements; +using Microsoft.Agents.A365.DevTools.Cli.Services.Requirements.RequirementChecks; using Microsoft.Agents.A365.DevTools.Cli.Services.Internal; using Microsoft.Extensions.Logging; using Microsoft.Graph; @@ -58,64 +61,32 @@ internal static class BlueprintSubcommand { // Client secret validation constants private const int ClientSecretValidationMaxRetries = 2; - private const int ClientSecretValidationRetryDelayMs = 1000; - 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. + /// Returns the requirement checks for setup blueprint. + /// Composes SetupCommand base checks + Location + ClientApp. /// - public static async Task> ValidateAsync( - Models.Agent365Config config, - IAzureValidator azureValidator, - IClientAppValidator clientAppValidator, - CancellationToken cancellationToken = default) + public static List GetChecks( + AzureAuthValidator auth, + IClientAppValidator clientAppValidator) { - var errors = new List(); - - if (string.IsNullOrWhiteSpace(config.ClientAppId)) + var checks = new List(SetupCommand.GetBaseChecks(auth)) { - 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."); - } + new LocationRequirementCheck(), + new ClientAppRequirementCheck(clientAppValidator), + }; - return errors; + return checks; } + private const int ClientSecretValidationRetryDelayMs = 1000; + private const int ClientSecretValidationTimeoutSeconds = 10; + private const string MicrosoftLoginOAuthTokenEndpoint = "https://login.microsoftonline.com/{0}/oauth2/v2.0/token"; public static Command CreateCommand( ILogger logger, IConfigService configService, CommandExecutor executor, - IAzureValidator azureValidator, + AzureAuthValidator authValidator, PlatformDetector platformDetector, IBotConfigurator botConfigurator, GraphApiService graphApiService, @@ -170,7 +141,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( @@ -229,21 +200,11 @@ await UpdateEndpointAsync( { try { - var requirementsResult = await RequirementsSubcommand.RunRequirementChecksAsync( - RequirementsSubcommand.GetRequirementChecks(clientAppValidator), - 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."); - logger.LogError("Use the resolution guidance provided for each failed check."); - ExceptionHandler.ExitWithCleanup(1); - } + var checks = BlueprintSubcommand.GetChecks(authValidator, clientAppValidator); + await RequirementsSubcommand.RunChecksOrExitAsync( + checks, setupConfig, logger, CancellationToken.None); } - catch (Exception reqEx) + catch (Exception reqEx) when (reqEx is not OperationCanceledException) { logger.LogError(reqEx, "Requirements check failed with an unexpected error: {Message}", reqEx.Message); logger.LogError("If you want to bypass requirement validation, rerun this command with the --skip-requirements flag."); @@ -265,6 +226,8 @@ await UpdateEndpointAsync( return; } + logger.LogInformation("Starting blueprint setup... (TraceId: {TraceId})", correlationId); + // Handle --endpoint-only flag if (endpointOnly) { @@ -302,7 +265,7 @@ await CreateBlueprintImplementationAsync( setupConfig, config, executor, - azureValidator, + authValidator, logger, false, false, @@ -366,7 +329,7 @@ public static async Task CreateBlueprintImplementationA Models.Agent365Config setupConfig, FileInfo config, CommandExecutor executor, - IAzureValidator azureValidator, + AzureAuthValidator authValidator, ILogger logger, bool skipInfrastructure, bool isSetupAll, @@ -399,17 +362,6 @@ public static async Task CreateBlueprintImplementationA logger.LogInformation(""); logger.LogInformation("==> Creating Agent Blueprint"); - // Validate Azure authentication - if (!await azureValidator.ValidateAllAsync(setupConfig.SubscriptionId)) - { - return new BlueprintCreationResult - { - BlueprintCreated = false, - EndpointRegistered = false, - EndpointRegistrationAttempted = false - }; - } - var generatedConfigPath = Path.Combine( config.DirectoryName ?? Environment.CurrentDirectory, "a365.generated.config.json"); @@ -1183,7 +1135,8 @@ public static async Task EnsureDelegatedConsentWithRetriesAsync( tenantId, objectId, userObjectId: null, - ct); + ct, + scopes: AuthenticationConstants.RequiredClientAppPermissions); if (isOwner) { @@ -1255,14 +1208,7 @@ await retryHelper.ExecuteWithRetryAsync( } else { - logger.LogError("Failed to create Federated Identity Credential: {Error}", ficCreateResult?.ErrorMessage ?? "Unknown error"); - logger.LogError("The agent instance may not be able to authenticate using Managed Identity"); - } - - if (!ficSuccess) - { - logger.LogWarning("Federated Identity Credential configuration incomplete"); - logger.LogWarning("You may need to create the credential manually in Entra ID"); + logger.LogWarning("[WARN] Federated Identity Credential creation failed - you may need to create it manually in Entra ID"); } } else if (!useManagedIdentity) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/CopilotStudioSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/CopilotStudioSubcommand.cs index 15539f92..ffa5c27b 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/CopilotStudioSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/CopilotStudioSubcommand.cs @@ -1,11 +1,14 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using Microsoft.Agents.A365.DevTools.Cli.Commands; using Microsoft.Agents.A365.DevTools.Cli.Constants; using Microsoft.Agents.A365.DevTools.Cli.Exceptions; 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.Extensions.Logging; using System.CommandLine; @@ -17,6 +20,12 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Commands.SetupSubcommands; /// internal static class CopilotStudioSubcommand { + /// + /// Returns the requirement checks for setup permissions copilotstudio. + /// + public static List GetChecks(AzureAuthValidator auth) + => SetupCommand.GetBaseChecks(auth); + /// /// Validates CopilotStudio permissions prerequisites without performing any actions. /// @@ -30,6 +39,7 @@ public static Task> ValidateAsync( public static Command CreateCommand( ILogger logger, + AzureAuthValidator authValidator, IConfigService configService, CommandExecutor executor, GraphApiService graphApiService, @@ -78,13 +88,8 @@ public static Command CreateCommand( // which would be a side effect in a mode that is supposed to be non-mutating. if (!dryRun) { - var systemChecksOk = await RequirementsSubcommand.RunRequirementChecksAsync( - RequirementsSubcommand.GetSystemRequirementChecks(), setupConfig, logger, category: null, CancellationToken.None); - if (!systemChecksOk) - { - logger.LogError("Setup cannot proceed due to failed requirement checks above. Please fix the issues and retry."); - ExceptionHandler.ExitWithCleanup(1); - } + var copilotChecks = CopilotStudioSubcommand.GetChecks(authValidator); + await RequirementsSubcommand.RunChecksOrExitAsync(copilotChecks, setupConfig, logger, CancellationToken.None); } if (dryRun) 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 7731bf0e..cbb2f753 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,8 @@ 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.Agents.A365.DevTools.Cli.Services.Requirements.RequirementChecks; using Microsoft.Extensions.Logging; using System.CommandLine; using System.Text.Json; @@ -24,84 +26,21 @@ public static class InfrastructureSubcommand private const int MaxSdkValidationAttempts = 3; 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. + /// Requirement checks for setup infrastructure: Azure auth, Frontier Preview, PowerShell modules, and infrastructure config. /// - private static bool IsValidAppServicePlanSku(string sku) + internal static List GetChecks(AzureAuthValidator auth) { - 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); + var checks = SetupCommand.GetBaseChecks(auth); + checks.Add(new InfrastructureRequirementCheck()); + return checks; } + public static Command CreateCommand( ILogger logger, IConfigService configService, - IAzureValidator azureValidator, + AzureAuthValidator authValidator, PlatformDetector platformDetector, CommandExecutor executor) { @@ -157,11 +96,8 @@ 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)) - { - ExceptionHandler.ExitWithCleanup(1); - } + await RequirementsSubcommand.RunChecksOrExitAsync( + GetChecks(authValidator), setupConfig, logger, CancellationToken.None); } else { diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs index f1870472..5a84c66b 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs @@ -1,11 +1,14 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using Microsoft.Agents.A365.DevTools.Cli.Commands; using Microsoft.Agents.A365.DevTools.Cli.Constants; using Microsoft.Agents.A365.DevTools.Cli.Exceptions; 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.Extensions.Logging; using System.CommandLine; using System.Threading; @@ -18,8 +21,27 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Commands.SetupSubcommands; /// internal static class PermissionsSubcommand { + /// + /// Returns the requirement checks for setup permissions mcp. + /// + public static List GetMcpChecks(AzureAuthValidator auth) + => SetupCommand.GetBaseChecks(auth); + + /// + /// Returns the requirement checks for setup permissions bot. + /// + public static List GetBotChecks(AzureAuthValidator auth) + => SetupCommand.GetBaseChecks(auth); + + /// + /// Returns the requirement checks for setup permissions custom. + /// + public static List GetCustomChecks(AzureAuthValidator auth) + => SetupCommand.GetBaseChecks(auth); + public static Command CreateCommand( ILogger logger, + AzureAuthValidator authValidator, IConfigService configService, CommandExecutor executor, GraphApiService graphApiService, @@ -30,10 +52,10 @@ public static Command CreateCommand( "Minimum required permissions: Global Administrator\n"); // Add subcommands - permissionsCommand.AddCommand(CreateMcpSubcommand(logger, configService, executor, graphApiService, blueprintService)); - permissionsCommand.AddCommand(CreateBotSubcommand(logger, configService, executor, graphApiService, blueprintService)); - permissionsCommand.AddCommand(CreateCustomSubcommand(logger, configService, executor, graphApiService, blueprintService)); - permissionsCommand.AddCommand(CopilotStudioSubcommand.CreateCommand(logger, configService, executor, graphApiService, blueprintService)); + permissionsCommand.AddCommand(CreateMcpSubcommand(logger, authValidator, configService, executor, graphApiService, blueprintService)); + permissionsCommand.AddCommand(CreateBotSubcommand(logger, authValidator, configService, executor, graphApiService, blueprintService)); + permissionsCommand.AddCommand(CreateCustomSubcommand(logger, authValidator, configService, executor, graphApiService, blueprintService)); + permissionsCommand.AddCommand(CopilotStudioSubcommand.CreateCommand(logger, authValidator, configService, executor, graphApiService, blueprintService)); return permissionsCommand; } @@ -43,6 +65,7 @@ public static Command CreateCommand( /// private static Command CreateMcpSubcommand( ILogger logger, + AzureAuthValidator authValidator, IConfigService configService, CommandExecutor executor, GraphApiService graphApiService, @@ -90,13 +113,8 @@ private static Command CreateMcpSubcommand( // which would be a side effect in a mode that is supposed to be non-mutating. if (!dryRun) { - var mcpSystemChecksOk = await RequirementsSubcommand.RunRequirementChecksAsync( - RequirementsSubcommand.GetSystemRequirementChecks(), setupConfig, logger, category: null, CancellationToken.None); - if (!mcpSystemChecksOk) - { - logger.LogError("Setup cannot proceed due to failed requirement checks above. Please fix the issues and retry."); - ExceptionHandler.ExitWithCleanup(1); - } + var mcpChecks = GetMcpChecks(authValidator); + await RequirementsSubcommand.RunChecksOrExitAsync(mcpChecks, setupConfig, logger, CancellationToken.None); } if (dryRun) @@ -133,6 +151,7 @@ await ConfigureMcpPermissionsAsync( /// private static Command CreateBotSubcommand( ILogger logger, + AzureAuthValidator authValidator, IConfigService configService, CommandExecutor executor, GraphApiService graphApiService, @@ -182,13 +201,8 @@ private static Command CreateBotSubcommand( // which would be a side effect in a mode that is supposed to be non-mutating. if (!dryRun) { - var botSystemChecksOk = await RequirementsSubcommand.RunRequirementChecksAsync( - RequirementsSubcommand.GetSystemRequirementChecks(), setupConfig, logger, category: null, CancellationToken.None); - if (!botSystemChecksOk) - { - logger.LogError("Setup cannot proceed due to failed requirement checks above. Please fix the issues and retry."); - ExceptionHandler.ExitWithCleanup(1); - } + var botChecks = GetBotChecks(authValidator); + await RequirementsSubcommand.RunChecksOrExitAsync(botChecks, setupConfig, logger, CancellationToken.None); } if (dryRun) @@ -222,6 +236,7 @@ await ConfigureBotPermissionsAsync( /// private static Command CreateCustomSubcommand( ILogger logger, + AzureAuthValidator authValidator, IConfigService configService, CommandExecutor executor, GraphApiService graphApiService, @@ -270,13 +285,8 @@ private static Command CreateCustomSubcommand( // which would be a side effect in a mode that is supposed to be non-mutating. if (!dryRun) { - var customSystemChecksOk = await RequirementsSubcommand.RunRequirementChecksAsync( - RequirementsSubcommand.GetSystemRequirementChecks(), setupConfig, logger, category: null, CancellationToken.None); - if (!customSystemChecksOk) - { - logger.LogError("Setup cannot proceed due to failed requirement checks above. Please fix the issues and retry."); - ExceptionHandler.ExitWithCleanup(1); - } + var customChecks = GetCustomChecks(authValidator); + await RequirementsSubcommand.RunChecksOrExitAsync(customChecks, setupConfig, logger, CancellationToken.None); } if (dryRun) 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..788f4e26 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/RequirementsSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/RequirementsSubcommand.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using Microsoft.Agents.A365.DevTools.Cli.Exceptions; using Microsoft.Agents.A365.DevTools.Cli.Models; using Microsoft.Agents.A365.DevTools.Cli.Services; using Microsoft.Agents.A365.DevTools.Cli.Services.Requirements; @@ -19,6 +20,7 @@ internal static class RequirementsSubcommand public static Command CreateCommand( ILogger logger, IConfigService configService, + AzureAuthValidator authValidator, IClientAppValidator clientAppValidator) { var command = new Command("requirements", @@ -57,7 +59,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 +103,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,41 +130,39 @@ 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(); + logger.LogInformation("Requirements: {Passed} passed, {Warning} warnings, {Failed} failed", + passedChecks, warningChecks, failedChecks); - 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 + return failedChecks == 0; + } + + /// + /// Runs checks with formatted [PASS]/[FAIL] output and exits if any fail. + /// Use this instead of RunRequirementChecksAsync when failure should abort the command. + /// + public static async Task RunChecksOrExitAsync( + List checks, + Agent365Config config, + ILogger logger, + CancellationToken cancellationToken = default) + { + var passed = await RunRequirementChecksAsync(checks, config, logger, category: null, cancellationToken); + if (!passed) { - logger.LogInformation("All requirements passed! You're ready to run Agent 365 setup."); + logger.LogError("Operation cannot proceed due to failed requirement checks above. Please fix the issues and retry."); + ExceptionHandler.ExitWithCleanup(1); } - - return failedChecks == 0; } /// /// 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(); } @@ -171,7 +170,7 @@ public static List GetRequirementChecks(IClientAppValidator c /// Gets system-level requirement checks that do not depend on configuration. /// These can be run before the configuration wizard to surface blockers early. /// - public static List GetSystemRequirementChecks() + private static List GetSystemRequirementChecks() { return new List { @@ -186,10 +185,13 @@ public static List GetSystemRequirementChecks() /// /// Gets configuration-dependent requirement checks that must run after the configuration is loaded. /// - public static List GetConfigRequirementChecks(IClientAppValidator clientAppValidator) + private 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/Exceptions/ClientAppValidationException.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Exceptions/ClientAppValidationException.cs index e7c1c221..28d58d7a 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Exceptions/ClientAppValidationException.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Exceptions/ClientAppValidationException.cs @@ -106,6 +106,30 @@ public static ClientAppValidationException MissingAdminConsent(string clientAppI }); } + /// + /// Creates exception for when the Azure token was revoked by a security event (CAE). + /// + public static ClientAppValidationException TokenRevoked(string clientAppId) + { + return new ClientAppValidationException( + issueDescription: "Azure authentication token revoked — re-authentication required", + errorDetails: new List + { + "Your Azure CLI token has been revoked due to a security event (Continuous Access Evaluation).", + "This occurs when a password is changed, MFA is updated, or a conditional access policy fires." + }, + mitigationSteps: new List + { + "Run: az logout", + "Run: az login", + "Then retry the command." + }, + context: new Dictionary + { + ["clientAppId"] = clientAppId + }); + } + /// /// Creates exception for general validation failures with custom details. /// 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/Program.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs index 2b6e2087..08b2ce02 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs @@ -90,7 +90,7 @@ static async Task Main(string[] args) var configService = serviceProvider.GetRequiredService(); var executor = serviceProvider.GetRequiredService(); var authService = serviceProvider.GetRequiredService(); - var azureValidator = serviceProvider.GetRequiredService(); + var authValidator = serviceProvider.GetRequiredService(); var toolingService = serviceProvider.GetRequiredService(); // Get services needed by commands @@ -110,11 +110,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, platformDetector, graphApiService, agentBlueprintService, blueprintLookupService, federatedCredentialService, clientAppValidator)); + deploymentService, botConfigurator, authValidator, 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, authValidator, graphApiService, agentBlueprintService)); // Register ConfigCommand var configLoggerFactory = serviceProvider.GetRequiredService(); @@ -124,7 +124,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, authValidator)); rootCommand.AddCommand(PublishCommand.CreateCommand(publishLogger, configService, agentPublishService, graphApiService, agentBlueprintService, manifestTemplateService)); // Wrap all command handlers with exception handling @@ -227,13 +227,10 @@ 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 multi-platform deployment services services.AddSingleton(); 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..ae714363 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AzureAuthValidator.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AzureAuthValidator.cs @@ -26,25 +26,16 @@ 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 { // Check Azure CLI authentication by trying to get current account - var result = await _executor.ExecuteAsync("az", "account show --output json", captureOutput: true); - + var result = await _executor.ExecuteAsync("az", "account show --output json", captureOutput: true, suppressErrorLogging: true); + if (!result.Success) { - _logger.LogError("Azure CLI authentication required!"); - _logger.LogInformation(""); - _logger.LogInformation("Please run the following command to log in to Azure:"); - _logger.LogInformation(" az login"); - _logger.LogInformation(""); - _logger.LogInformation("After logging in, run this command again."); - _logger.LogInformation(""); - _logger.LogInformation("For more information about Azure CLI authentication:"); - _logger.LogInformation(" https://docs.microsoft.com/en-us/cli/azure/authenticate-azure-cli"); - _logger.LogInformation(""); + _logger.LogDebug("Azure CLI authentication check failed: {Error}", result.StandardError); return false; } @@ -66,13 +57,7 @@ public async Task ValidateAuthenticationAsync(string? expectedSubscription { if (!string.Equals(subscriptionId, expectedSubscriptionId, StringComparison.OrdinalIgnoreCase)) { - _logger.LogError("Azure CLI is using a different subscription than configured"); - _logger.LogError(" Expected: {ExpectedSubscription}", expectedSubscriptionId); - _logger.LogError(" Current: {CurrentSubscription}", subscriptionId); - _logger.LogInformation(""); - _logger.LogInformation("Please switch to the correct subscription:"); - _logger.LogInformation(" az account set --subscription {ExpectedSubscription}", expectedSubscriptionId); - _logger.LogInformation(""); + _logger.LogDebug("Subscription mismatch — expected: {Expected}, current: {Current}", expectedSubscriptionId, subscriptionId); return false; } @@ -92,4 +77,21 @@ public async Task ValidateAuthenticationAsync(string? expectedSubscription return false; } } + + /// + /// Probes the Azure App Service token scope to verify deployment credentials are valid. + /// Returns false if the grant is expired or revoked (AADSTS50173 / invalid_grant). + /// + public virtual async Task GetAppServiceTokenAsync(CancellationToken ct = default) + { + var result = await _executor.ExecuteAsync( + "az", + "account get-access-token --resource https://appservice.azure.com", + captureOutput: true, + suppressErrorLogging: true, + cancellationToken: ct); + + _logger.LogDebug("App Service token probe: {Result}", result.Success ? "valid" : "expired or revoked"); + return result.Success; + } } \ No newline at end of file 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/ClientAppValidator.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/ClientAppValidator.cs index bd724030..f2ae0cc5 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}"; @@ -325,6 +325,7 @@ private async Task EnsurePublicClientFlowsEnabledAsync( var tokenResult = await _executor.ExecuteAsync( "az", $"account get-access-token --resource {GraphTokenResource} --query accessToken -o tsv", + suppressErrorLogging: true, cancellationToken: ct); if (!tokenResult.Success || string.IsNullOrWhiteSpace(tokenResult.StandardOutput)) @@ -343,6 +344,7 @@ private async Task EnsurePublicClientFlowsEnabledAsync( var appCheckResult = await _executor.ExecuteAsync( "az", $"rest --method GET --url \"{GraphApiBaseUrl}/applications?$filter=appId eq '{CommandStringHelper.EscapePowerShellString(clientAppId)}'&$select=id,appId,displayName,requiredResourceAccess\" --headers \"Authorization=Bearer {CommandStringHelper.EscapePowerShellString(graphToken)}\"", + suppressErrorLogging: true, cancellationToken: ct); if (!appCheckResult.Success) @@ -351,12 +353,13 @@ private async Task EnsurePublicClientFlowsEnabledAsync( if (appCheckResult.StandardError.Contains("TokenCreatedWithOutdatedPolicies", StringComparison.OrdinalIgnoreCase) || appCheckResult.StandardError.Contains("InvalidAuthenticationToken", StringComparison.OrdinalIgnoreCase)) { - _logger.LogWarning("Azure CLI token is stale due to Continuous Access Evaluation. Refreshing token automatically..."); - + _logger.LogDebug("Azure CLI token is stale due to Continuous Access Evaluation. Attempting token refresh..."); + // Force token refresh var refreshResult = await _executor.ExecuteAsync( "az", $"account get-access-token --resource {GraphTokenResource} --query accessToken -o tsv", + suppressErrorLogging: true, cancellationToken: ct); if (refreshResult.Success && !string.IsNullOrWhiteSpace(refreshResult.StandardOutput)) @@ -368,6 +371,7 @@ private async Task EnsurePublicClientFlowsEnabledAsync( var retryResult = await _executor.ExecuteAsync( "az", $"rest --method GET --url \"{GraphApiBaseUrl}/applications?$filter=appId eq '{CommandStringHelper.EscapePowerShellString(clientAppId)}'&$select=id,appId,displayName,requiredResourceAccess\" --headers \"Authorization=Bearer {CommandStringHelper.EscapePowerShellString(freshToken)}\"", + suppressErrorLogging: true, cancellationToken: ct); if (retryResult.Success) @@ -376,14 +380,20 @@ private async Task EnsurePublicClientFlowsEnabledAsync( } else { + // Token refresh succeeded but the Graph call still rejected it — the revocation + // is server-side and cannot be silently recovered. Throw explicitly so the + // caller shows "token revoked" rather than "app not found". _logger.LogDebug("App query failed after token refresh: {Error}", retryResult.StandardError); - return null; + throw ClientAppValidationException.TokenRevoked(clientAppId); } } } if (!appCheckResult.Success) { + if (IsCaeError(appCheckResult.StandardError)) + throw ClientAppValidationException.TokenRevoked(clientAppId); + _logger.LogDebug("App query failed: {Error}", appCheckResult.StandardError); return null; } @@ -702,6 +712,11 @@ private async Task ValidateAdminConsentAsync(string clientAppId, string gr #region Helper Types + private static bool IsCaeError(string errorOutput) => + errorOutput.Contains("TokenIssuedBeforeRevocationTimestamp", StringComparison.OrdinalIgnoreCase) || + errorOutput.Contains("TokenCreatedWithOutdatedPolicies", StringComparison.OrdinalIgnoreCase) || + errorOutput.Contains("InvalidAuthenticationToken", StringComparison.OrdinalIgnoreCase); + private record ClientAppInfo(string ObjectId, string DisplayName, JsonArray? RequiredResourceAccess); #endregion diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/CommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/CommandExecutor.cs index ba47a82c..d9fa6a4d 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/CommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/CommandExecutor.cs @@ -115,6 +115,7 @@ public virtual async Task ExecuteWithStreamingAsync( string outputPrefix = "", bool interactive = false, Func? outputTransform = null, + bool suppressErrorLogging = false, CancellationToken cancellationToken = default) { _logger.LogDebug("Executing with streaming: {Command} {Arguments} (Interactive={Interactive})", command, arguments, interactive); @@ -178,10 +179,14 @@ public virtual async Task ExecuteWithStreamingAsync( errorBuilder.AppendLine(args.Data); // Azure CLI writes informational messages to stderr with "WARNING:" prefix // Strip it for cleaner output - var cleanData = IsAzureCliCommand(command) - ? StripAzureWarningPrefix(args.Data) + var cleanData = IsAzureCliCommand(command) + ? StripAzureWarningPrefix(args.Data) : args.Data; - Console.WriteLine($"{outputPrefix}{cleanData}"); + // Skip blank lines that result from stripping az cli prefixes + if (!string.IsNullOrWhiteSpace(cleanData)) + { + Console.WriteLine($"{outputPrefix}{cleanData}"); + } } }; @@ -200,7 +205,7 @@ public virtual async Task ExecuteWithStreamingAsync( StandardError = errorBuilder.ToString() }; - if (result.ExitCode != 0) + if (result.ExitCode != 0 && !suppressErrorLogging) { _logger.LogError("Command failed with exit code {ExitCode}", result.ExitCode); } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/ConfigService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/ConfigService.cs index f3ed912d..3996c603 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/ConfigService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/ConfigService.cs @@ -241,7 +241,6 @@ public async Task LoadAsync( // Validate static config file exists if (!File.Exists(resolvedConfigPath)) { - _logger?.LogError("Static configuration file not found: {ConfigPath}", resolvedConfigPath); throw new ConfigFileNotFoundException(resolvedConfigPath); } 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) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/DeploymentService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/DeploymentService.cs index 7cdf8275..a1ee94f9 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/DeploymentService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/DeploymentService.cs @@ -183,57 +183,39 @@ private async Task DeployToAzureAsync(DeploymentConfiguration config, string pro // Explicitly set the correct runtime configuration before deployment await EnsureCorrectRuntimeConfigurationAsync(config.ResourceGroup, config.AppName, platform, projectDir); - _logger.LogInformation("Deployment typically takes 2-5 minutes to complete"); _logger.LogDebug("Using async deployment to avoid Azure SCM gateway timeout (4-5 minute limit)"); - _logger.LogInformation("Monitor progress: https://{AppName}.scm.azurewebsites.net/api/deployments/latest", config.AppName); - _logger.LogInformation(""); var deployArgs = $"webapp deploy --resource-group {config.ResourceGroup} --name {config.AppName} --src-path \"{zipPath}\" --type zip --async true"; _logger.LogInformation("Uploading deployment package..."); - var deployResult = await _executor.ExecuteWithStreamingAsync("az", deployArgs, projectDir, "[Azure] "); + var deployResult = await _executor.ExecuteWithStreamingAsync("az", deployArgs, projectDir, "[Azure] ", suppressErrorLogging: true); if (!deployResult.Success) { - _logger.LogError("Deployment upload failed with exit code {ExitCode}", deployResult.ExitCode); - if (!string.IsNullOrWhiteSpace(deployResult.StandardError)) - { - _logger.LogError("Deployment error: {Error}", deployResult.StandardError); + bool isSiteStartTimeout = + deployResult.StandardError.Contains("site failed to start within 10 mins", StringComparison.OrdinalIgnoreCase) || + deployResult.StandardError.Contains("worker proccess failed to start", StringComparison.OrdinalIgnoreCase) || + deployResult.StandardError.Contains("worker process failed to start", StringComparison.OrdinalIgnoreCase); - // Graceful handling for site start timeout - if (deployResult.StandardError.Contains("site failed to start within 10 mins", StringComparison.OrdinalIgnoreCase) || - deployResult.StandardError.Contains("worker proccess failed to start", StringComparison.OrdinalIgnoreCase)) - { - _logger.LogError("The deployment failed because the site did not start within the expected time."); - _logger.LogError("This is often caused by application startup issues, missing dependencies, or misconfiguration."); - _logger.LogError("Check the runtime logs for more details: https://{AppName}.scm.azurewebsites.net/api/logs/docker", config.AppName); - _logger.LogError("Common causes include:"); - _logger.LogError(" - Incorrect startup command or entry point"); - _logger.LogError(" - Missing Python/Node/.NET dependencies"); - _logger.LogError(" - Application errors on startup"); - _logger.LogError(" - Port binding issues (ensure your app listens on the correct port)"); - _logger.LogError(" - Long initialization times"); - _logger.LogError("Review your application logs and configuration, then redeploy."); - } + if (isSiteStartTimeout) + { + _logger.LogInformation(""); + _logger.LogInformation("Common causes for site startup failure:"); + _logger.LogInformation(" - Incorrect startup command or entry point"); + _logger.LogInformation(" - Missing dependencies"); + _logger.LogInformation(" - Application errors on startup"); + _logger.LogInformation(" - Port binding issues (app must listen on the port set by the PORT environment variable)"); + throw new DeployAppException( + $"Site failed to start within the allotted time. Check runtime logs: https://{config.AppName}.scm.azurewebsites.net/api/logs/docker"); } - // Print a summary for the user - _logger.LogInformation("========================================"); - _logger.LogInformation("Deployment Summary"); - _logger.LogInformation("App Name: {AppName}", config.AppName); - _logger.LogInformation("App URL: https://{AppName}.azurewebsites.net", config.AppName); - _logger.LogInformation("Resource Group: {ResourceGroup}", config.ResourceGroup); - _logger.LogInformation("Deployment failed. See error details above."); - _logger.LogInformation("========================================"); - - throw new DeployAppException($"Azure deployment failed: {deployResult.StandardError}"); + throw new DeployAppException($"az webapp deploy failed with exit code {deployResult.ExitCode}"); } _logger.LogInformation(""); _logger.LogInformation("Deployment package uploaded successfully!"); _logger.LogInformation(""); - _logger.LogInformation("Deployment is continuing in the background on Azure"); - _logger.LogInformation("Application will be available in 2-5 minutes"); + _logger.LogInformation("Build and startup are running on Azure. This may take several minutes."); _logger.LogInformation(""); _logger.LogInformation("Monitor deployment status:"); _logger.LogInformation(" Web: https://{AppName}.scm.azurewebsites.net/api/deployments/latest", config.AppName); @@ -257,7 +239,7 @@ private async Task DeployToAzureAsync(DeploymentConfiguration config, string pro private async Task CreateDeploymentPackageAsync(string projectDir, string publishPath, string deploymentZipName) { var zipPath = Path.Combine(projectDir, deploymentZipName); - _logger.LogInformation("[6/7] Creating deployment package: {ZipPath}", zipPath); + _logger.LogInformation("Creating deployment package: {ZipPath}", zipPath); // Delete old zip if exists with retry logic if (File.Exists(zipPath)) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/FederatedCredentialService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/FederatedCredentialService.cs index 2873bbf2..c6bcadcd 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/FederatedCredentialService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/FederatedCredentialService.cs @@ -316,14 +316,13 @@ public async Task CreateFederatedCredentialAsyn continue; } - // Both endpoints failed - _logger.LogError("Failed to create federated credential: HTTP {StatusCode} {ReasonPhrase}", response.StatusCode, response.ReasonPhrase); - if (!string.IsNullOrWhiteSpace(response.Body)) - { - _logger.LogError("Error details: {Body}", response.Body); - } - - _logger.LogError("Failed to create federated credential: {Name}", name); + // Both endpoints failed — log one clean error + var graphError = TryExtractGraphErrorMessage(response.Body); + if (graphError != null) + _logger.LogError("Failed to create federated credential '{Name}': {ErrorMessage}", name, graphError); + else + _logger.LogError("Failed to create federated credential '{Name}': HTTP {StatusCode} {ReasonPhrase}", name, response.StatusCode, response.ReasonPhrase); + _logger.LogDebug("Federated credential error response body: {Body}", response.Body); return new FederatedCredentialCreateResult { Success = false, @@ -471,4 +470,20 @@ public async Task DeleteAllFederatedCredentialsAsync( return false; } } + + private static string? TryExtractGraphErrorMessage(string? body) + { + if (string.IsNullOrWhiteSpace(body)) return null; + try + { + using var doc = JsonDocument.Parse(body); + if (doc.RootElement.TryGetProperty("error", out var error) && + error.TryGetProperty("message", out var msg)) + { + return msg.GetString(); + } + } + catch { /* ignore parse errors */ } + return null; + } } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs index de3cbdad..82d52547 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs @@ -307,7 +307,12 @@ private async Task EnsureGraphHeadersAsync(string tenantId, CancellationTo var body = await resp.Content.ReadAsStringAsync(ct); if (!resp.IsSuccessStatusCode) { - _logger.LogError("Graph POST {Url} failed {Code} {Reason}: {Body}", url, (int)resp.StatusCode, resp.ReasonPhrase, body); + var errorMessage = TryExtractGraphErrorMessage(body); + if (errorMessage != null) + _logger.LogError("Graph POST {Url} failed: {ErrorMessage}", url, errorMessage); + else + _logger.LogError("Graph POST {Url} failed {Code} {Reason}", url, (int)resp.StatusCode, resp.ReasonPhrase); + _logger.LogDebug("Graph POST response body: {Body}", body); return null; } @@ -366,7 +371,12 @@ public virtual async Task GraphPatchAsync(string tenantId, string relative if (!resp.IsSuccessStatusCode) { var body = await resp.Content.ReadAsStringAsync(ct); - _logger.LogError("Graph PATCH {Url} failed {Code} {Reason}: {Body}", url, (int)resp.StatusCode, resp.ReasonPhrase, body); + var errorMessage = TryExtractGraphErrorMessage(body); + if (errorMessage != null) + _logger.LogError("Graph PATCH {Url} failed: {ErrorMessage}", url, errorMessage); + else + _logger.LogError("Graph PATCH {Url} failed {Code} {Reason}", url, (int)resp.StatusCode, resp.ReasonPhrase); + _logger.LogDebug("Graph PATCH response body: {Body}", body); } return resp.IsSuccessStatusCode; @@ -394,7 +404,12 @@ public async Task GraphDeleteAsync( if (!resp.IsSuccessStatusCode) { var body = await resp.Content.ReadAsStringAsync(ct); - _logger.LogError("Graph DELETE {Url} failed {Code} {Reason}: {Body}", url, (int)resp.StatusCode, resp.ReasonPhrase, body); + var errorMessage = TryExtractGraphErrorMessage(body); + if (errorMessage != null) + _logger.LogError("Graph DELETE {Url} failed: {ErrorMessage}", url, errorMessage); + else + _logger.LogError("Graph DELETE {Url} failed {Code} {Reason}", url, (int)resp.StatusCode, resp.ReasonPhrase); + _logger.LogDebug("Graph DELETE response body: {Body}", body); return false; } @@ -672,4 +687,24 @@ public virtual async Task IsApplicationOwnerAsync( return false; } } + + /// + /// Attempts to extract a human-readable error message from a Graph API JSON error response body. + /// Returns null if the body cannot be parsed or does not contain an error message. + /// + private static string? TryExtractGraphErrorMessage(string body) + { + if (string.IsNullOrWhiteSpace(body)) return null; + try + { + using var doc = JsonDocument.Parse(body); + if (doc.RootElement.TryGetProperty("error", out var error) && + error.TryGetProperty("message", out var msg)) + { + return msg.GetString(); + } + } + catch { /* ignore parse errors */ } + return null; + } } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Helpers/CleanConsoleFormatter.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Helpers/CleanConsoleFormatter.cs index dac31d7c..0b96915f 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Helpers/CleanConsoleFormatter.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Helpers/CleanConsoleFormatter.cs @@ -64,14 +64,12 @@ public override void Write( if (isConsole) { Console.ForegroundColor = ConsoleColor.Yellow; - Console.Write("WARNING: "); Console.Write(message); Console.ResetColor(); Console.WriteLine(); } else { - textWriter.Write("WARNING: "); textWriter.WriteLine(message); } break; 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/NodeBuilder.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/NodeBuilder.cs index ae8dc1d2..f179b097 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/NodeBuilder.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/NodeBuilder.cs @@ -51,7 +51,7 @@ public async Task ValidateEnvironmentAsync() public async Task CleanAsync(string projectDir) { - _logger.LogInformation("Cleaning Node.js project..."); + _logger.LogDebug("Cleaning Node.js publish output..."); // Remove node_modules if it exists var nodeModulesPath = Path.Combine(projectDir, "node_modules"); 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/PrerequisiteRunner.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/PrerequisiteRunner.cs new file mode 100644 index 00000000..6b9422b6 --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/PrerequisiteRunner.cs @@ -0,0 +1,37 @@ +// 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 aggregates pass/fail results. +/// Each check handles its own [PASS]/[FAIL]/[WARN] logging via ExecuteCheckWithLoggingAsync. +/// +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; + } + } + + return passed; + } +} 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..7938ee6b 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementCheck.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementCheck.cs @@ -23,36 +23,22 @@ 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})"); } /// /// Helper method to log check warning /// - protected virtual void LogCheckWarning(ILogger logger, string? details = null) + protected virtual void LogCheckWarning(ILogger logger, string? message = null) { - logger.LogWarning("[WARNING] {Name}: Cannot automatically verify", Name); - if (!string.IsNullOrWhiteSpace(details)) - { - logger.LogWarning(" Details: {Details}", details); - } + logger.LogWarning("[WARN] {Name}{Details}", Name, + string.IsNullOrWhiteSpace(message) ? "" : $" - {message}"); } /// @@ -60,7 +46,7 @@ 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("[FAIL] {Name}", Name); logger.LogError(" Issue: {ErrorMessage}", errorMessage); logger.LogError(" Resolution: {ResolutionGuidance}", resolutionGuidance); } @@ -74,7 +60,6 @@ protected async Task ExecuteCheckWithLoggingAsync( Func> checkImplementation, CancellationToken cancellationToken = default) { - LogCheckStart(logger); try { @@ -84,7 +69,10 @@ protected async Task ExecuteCheckWithLoggingAsync( { if (result.IsWarning) { - LogCheckWarning(logger, result.Details); + var warningMessage = (!string.IsNullOrWhiteSpace(result.ErrorMessage) && !string.IsNullOrWhiteSpace(result.Details)) + ? $"{result.ErrorMessage} - {result.Details}" + : result.ErrorMessage ?? result.Details; + LogCheckWarning(logger, warningMessage); } else { diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/AppServiceAuthRequirementCheck.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/AppServiceAuthRequirementCheck.cs new file mode 100644 index 00000000..be80cf8d --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/AppServiceAuthRequirementCheck.cs @@ -0,0 +1,53 @@ +// 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 that the Azure App Service deployment token is valid and not expired or revoked. +/// Probes the App Service token scope explicitly, which is not covered by AzureAuthRequirementCheck. +/// Catches stale/revoked grants (AADSTS50173) before build and upload begin. +/// +public class AppServiceAuthRequirementCheck : RequirementCheck +{ + private readonly AzureAuthValidator _auth; + + public AppServiceAuthRequirementCheck(AzureAuthValidator auth) + { + _auth = auth ?? throw new ArgumentNullException(nameof(auth)); + } + + /// + public override string Name => "App Service Authentication"; + + /// + public override string Description => "Validates that the Azure App Service deployment token is valid and not expired or revoked"; + + /// + public override string Category => "Azure"; + + /// + public override async Task CheckAsync( + Agent365Config config, + ILogger logger, + CancellationToken cancellationToken = default) + { + return await ExecuteCheckWithLoggingAsync(config, logger, CheckImplementationAsync, cancellationToken); + } + + private async Task CheckImplementationAsync( + Agent365Config config, + ILogger logger, + CancellationToken cancellationToken) + { + var success = await _auth.GetAppServiceTokenAsync(cancellationToken); + return success + ? RequirementCheckResult.Success() + : RequirementCheckResult.Failure( + "Azure App Service token is expired or revoked", + "Run 'az logout' then 'az login --tenant ' and retry"); + } +} 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..ef0e7c96 --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/AzureAuthRequirementCheck.cs @@ -0,0 +1,56 @@ +// 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) + { + 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) + { + 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/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/InfrastructureRequirementCheck.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/InfrastructureRequirementCheck.cs new file mode 100644 index 00000000..960e3a11 --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/InfrastructureRequirementCheck.cs @@ -0,0 +1,101 @@ +// 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) + { + return ExecuteCheckWithLoggingAsync(config, logger, CheckImplementationAsync, cancellationToken); + } + + private static Task CheckImplementationAsync( + Agent365Config config, + ILogger logger, + CancellationToken cancellationToken) + { + 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), I1/I2/I3/I1V2/I2V2/I3V2 (Isolated)"); + + 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/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/MosPrerequisitesRequirementCheck.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/MosPrerequisitesRequirementCheck.cs new file mode 100644 index 00000000..55f60104 --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/MosPrerequisitesRequirementCheck.cs @@ -0,0 +1,72 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Microsoft.Agents.A365.DevTools.Cli.Exceptions; +using Microsoft.Agents.A365.DevTools.Cli.Helpers; +using Microsoft.Agents.A365.DevTools.Cli.Models; +using Microsoft.Extensions.Logging; + +namespace Microsoft.Agents.A365.DevTools.Cli.Services.Requirements.RequirementChecks; + +/// +/// Ensures MOS service principals exist in the tenant, creating and configuring them if absent. +/// Wraps PublishHelpers.EnsureMosPrerequisitesAsync so it runs before the interactive manifest +/// editing pause, preventing wasted work if MOS prerequisites are not configured. +/// +public class MosPrerequisitesRequirementCheck : RequirementCheck +{ + private readonly GraphApiService _graphApiService; + private readonly AgentBlueprintService _blueprintService; + + public MosPrerequisitesRequirementCheck(GraphApiService graphApiService, AgentBlueprintService blueprintService) + { + _graphApiService = graphApiService ?? throw new ArgumentNullException(nameof(graphApiService)); + _blueprintService = blueprintService ?? throw new ArgumentNullException(nameof(blueprintService)); + } + + /// + public override string Name => "MOS Prerequisites"; + + /// + public override string Description => "Ensures MOS service principals exist in tenant, creating and configuring them if absent"; + + /// + public override string Category => "MOS"; + + /// + public override async Task CheckAsync( + Agent365Config config, + ILogger logger, + CancellationToken cancellationToken = default) + { + return await ExecuteCheckWithLoggingAsync(config, logger, CheckImplementationAsync, cancellationToken); + } + + private async Task CheckImplementationAsync( + Agent365Config config, + ILogger logger, + CancellationToken cancellationToken) + { + try + { + var ok = await PublishHelpers.EnsureMosPrerequisitesAsync( + _graphApiService, _blueprintService, config, logger, cancellationToken); + + return ok + ? RequirementCheckResult.Success() + : RequirementCheckResult.Failure( + "MOS service principals not configured", + "Run 'a365 setup all' to configure MOS prerequisites"); + } + catch (SetupValidationException ex) + { + // EnsureMosPrerequisitesAsync throws SetupValidationException for unrecoverable + // failures (e.g., insufficient privileges). Convert to Failure so the check + // framework returns [FAIL] with guidance rather than an unhandled exception. + var resolution = ex.MitigationSteps.Count > 0 + ? string.Join("\n", ex.MitigationSteps) + : "Run 'a365 setup all' to configure MOS prerequisites"; + return RequirementCheckResult.Failure(ex.Message, resolution); + } + } +} 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..3720d340 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,18 +88,18 @@ 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)) ); } // Attempt auto-install for missing modules - logger.LogInformation("Attempting to auto-install missing PowerShell modules..."); + logger.LogDebug("Attempting to auto-install missing PowerShell modules..."); var autoInstalled = new List(); var stillMissing = new List(); foreach (var module in missingModules) { - logger.LogInformation("Installing {ModuleName}...", module.Name); + logger.LogDebug("Installing {ModuleName}...", module.Name); var installSuccess = await InstallModuleAsync(module.Name, logger, cancellationToken); if (installSuccess) @@ -111,12 +108,12 @@ private async Task CheckImplementationAsync(Agent365Conf if (verified) { autoInstalled.Add(module); - logger.LogInformation("Successfully installed {ModuleName}", module.Name); + logger.LogDebug("Successfully installed {ModuleName}", module.Name); } else { stillMissing.Add(module); - logger.LogWarning("Install succeeded but {ModuleName} not found in module path after install", module.Name); + logger.LogDebug("Install succeeded but {ModuleName} not found in module path after install", module.Name); } } else diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/design.md b/src/Microsoft.Agents.A365.DevTools.Cli/design.md index b2da814a..a0be376d 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/design.md +++ b/src/Microsoft.Agents.A365.DevTools.Cli/design.md @@ -33,7 +33,13 @@ Microsoft.Agents.A365.DevTools.Cli/ │ ├── BotConfigurator.cs # Messaging endpoint registration │ ├── GraphApiService.cs # Graph API interactions │ ├── AuthenticationService.cs # MSAL.NET authentication -│ └── Helpers/ # Service helper utilities +│ ├── AzureAuthValidator.cs # Azure CLI auth + App Service token validation +│ ├── Helpers/ # Service helper utilities +│ └── Requirements/ # Prerequisite validation system +│ ├── IRequirementCheck.cs # Check interface +│ ├── RequirementCheck.cs # Abstract base class with logging wrapper +│ ├── RequirementCheckResult.cs # Success/Warning/Failure result +│ └── RequirementChecks/ # Concrete check implementations ├── Models/ # Data models │ ├── Agent365Config.cs # Unified configuration model │ ├── ProjectPlatform.cs # Platform enumeration @@ -200,6 +206,78 @@ public class SetupCommand : AsyncCommand --- +## Prerequisite Validation Pattern (IRequirementCheck) + +Commands validate prerequisites through a structured check system before performing any mutating work. This produces consistent `[PASS]`/`[FAIL]`/`[WARN]` output and ensures users see actionable errors early. + +### Core Types + +```csharp +// Each check returns a structured result +public class RequirementCheckResult +{ + public bool Passed { get; } // true = pass or warning, false = failure + public bool IsWarning { get; } // true = warning (non-blocking) + public string? ErrorMessage { get; } // What went wrong + public string? ResolutionGuidance { get; } // How to fix it + public string? Details { get; } // Additional context (e.g., URLs) +} + +// Base class handles [PASS]/[FAIL]/[WARN] output and check execution +public abstract class RequirementCheck : IRequirementCheck +{ + public abstract string Name { get; } + public abstract string Category { get; } + public abstract Task CheckAsync(Agent365Config, ILogger, CancellationToken); +} +``` + +### Check Composition + +Each command declares its checks via a static `GetChecks()` method, making composition explicit and testable: + +```csharp +// deploy: auth first, then App Service token +public static List GetChecks(AzureAuthValidator auth) + => [new AzureAuthRequirementCheck(auth), new AppServiceAuthRequirementCheck(auth)]; + +// setup infrastructure: base checks + config validation +internal static List GetChecks(AzureAuthValidator auth) +{ + var checks = SetupCommand.GetBaseChecks(auth); // Auth + FrontierPreview + PowerShell + checks.Add(new InfrastructureRequirementCheck()); + return checks; +} +``` + +### Running Checks + +`RequirementsSubcommand.RunChecksOrExitAsync` is the shared runner — prints `[PASS]/[FAIL]/[WARN]` per check and calls `ExceptionHandler.ExitWithCleanup(1)` on any failure: + +```csharp +await RequirementsSubcommand.RunChecksOrExitAsync( + GetChecks(authValidator), config, logger, cancellationToken); +``` + +### Dry-Run Rule + +Commands supporting `--dry-run` skip checks entirely — the `RunChecksOrExitAsync` call is guarded by `if (!dryRun)` so dry runs are always fast and require no Azure credentials. + +### Available Checks + +| Check | Category | Used By | +|-------|----------|---------| +| `AzureAuthRequirementCheck` | Azure | setup all, setup infra, deploy, cleanup azure | +| `AppServiceAuthRequirementCheck` | Azure | deploy | +| `FrontierPreviewRequirementCheck` | Tenant Enrollment | setup all, setup infra | +| `PowerShellModulesRequirementCheck` | Tools | setup all, setup infra | +| `InfrastructureRequirementCheck` | Configuration | setup infra | +| `MosPrerequisitesRequirementCheck` | MOS | publish | +| `LocationRequirementCheck` | Configuration | setup endpoint | +| `ClientAppRequirementCheck` | Configuration | setup blueprint | + +--- + ## Multiplatform Deployment Architecture ### Platform Detection 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 5bfa5b45..79c84af2 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 @@ -7,6 +7,7 @@ using Microsoft.Agents.A365.DevTools.Cli.Services; using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using NSubstitute; using System.CommandLine; using System.CommandLine.Builder; @@ -26,7 +27,7 @@ public class BlueprintSubcommandTests private readonly ILogger _mockLogger; private readonly IConfigService _mockConfigService; private readonly CommandExecutor _mockExecutor; - private readonly IAzureValidator _mockAzureValidator; + private readonly AzureAuthValidator _mockAuthValidator; private readonly PlatformDetector _mockPlatformDetector; private readonly IBotConfigurator _mockBotConfigurator; private readonly GraphApiService _mockGraphApiService; @@ -41,7 +42,7 @@ public BlueprintSubcommandTests() _mockConfigService = Substitute.For(); var mockExecutorLogger = Substitute.For>(); _mockExecutor = Substitute.ForPartsOf(mockExecutorLogger); - _mockAzureValidator = Substitute.For(); + _mockAuthValidator = Substitute.ForPartsOf(NullLogger.Instance, _mockExecutor); var mockPlatformDetectorLogger = Substitute.For>(); _mockPlatformDetector = Substitute.ForPartsOf(mockPlatformDetectorLogger); _mockBotConfigurator = Substitute.For(); @@ -60,7 +61,7 @@ public void CreateCommand_ShouldHaveCorrectName() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockAuthValidator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -77,7 +78,7 @@ public void CreateCommand_ShouldHaveDescription() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockAuthValidator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -95,7 +96,7 @@ public void CreateCommand_ShouldHaveConfigOption() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockAuthValidator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -115,7 +116,7 @@ public void CreateCommand_ShouldHaveVerboseOption() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockAuthValidator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -135,7 +136,7 @@ public void CreateCommand_ShouldHaveDryRunOption() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockAuthValidator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -154,7 +155,7 @@ public void CreateCommand_ShouldHaveSkipRequirementsOption() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockAuthValidator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -182,7 +183,7 @@ public async Task DryRun_ShouldLoadConfigAndNotExecute() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockAuthValidator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -196,7 +197,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] @@ -216,7 +216,7 @@ public async Task DryRun_ShouldDisplayBlueprintInformation() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockAuthValidator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -252,20 +252,17 @@ public async Task CreateBlueprintImplementation_WithMissingDisplayName_ShouldThr var configFile = new FileInfo("test-config.json"); - _mockAzureValidator.ValidateAllAsync(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, + _mockAuthValidator, _mockLogger, skipInfrastructure: false, isSetupAll: false, @@ -280,44 +277,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"); - - _mockAzureValidator.ValidateAllAsync(Arg.Any()) - .Returns(false); // Validation fails - - // Act - var result = await BlueprintSubcommand.CreateBlueprintImplementationAsync( - config, - configFile, - _mockExecutor, - _mockAzureValidator, - _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 _mockAzureValidator.Received(1).ValidateAllAsync(config.SubscriptionId); - } [Fact] public void CommandDescription_ShouldMentionRequiredPermissions() @@ -327,7 +286,7 @@ public void CommandDescription_ShouldMentionRequiredPermissions() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockAuthValidator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -354,7 +313,7 @@ public async Task DryRun_WithCustomConfigPath_ShouldLoadCorrectFile() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockAuthValidator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -389,7 +348,7 @@ public async Task DryRun_ShouldNotCreateServicePrincipal() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockAuthValidator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -416,7 +375,7 @@ public void CreateCommand_ShouldHandleAllOptions() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockAuthValidator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -441,7 +400,7 @@ public async Task DryRun_WithMissingConfig_ShouldHandleGracefully() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockAuthValidator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -462,7 +421,7 @@ public void CreateCommand_DefaultConfigPath_ShouldBeA365ConfigJson() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockAuthValidator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -488,15 +447,12 @@ public async Task CreateBlueprintImplementation_ShouldLogProgressMessages() var configFile = new FileInfo("test-config.json"); - _mockAzureValidator.ValidateAllAsync(Arg.Any()) - .Returns(false); // Fail fast for this test - // Act var result = await BlueprintSubcommand.CreateBlueprintImplementationAsync( config, configFile, _mockExecutor, - _mockAzureValidator, + _mockAuthValidator, _mockLogger, skipInfrastructure: false, isSetupAll: false, @@ -527,7 +483,7 @@ public void CommandDescription_ShouldBeInformativeAndActionable() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockAuthValidator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -554,7 +510,7 @@ public async Task DryRun_WithVerboseFlag_ShouldSucceed() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockAuthValidator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -587,7 +543,7 @@ public async Task DryRun_ShouldShowWhatWouldBeDone() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockAuthValidator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -618,7 +574,7 @@ public void CreateCommand_ShouldBeUsableInCommandPipeline() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockAuthValidator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -1307,7 +1263,7 @@ public void CreateCommand_ShouldHaveUpdateEndpointOption() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockAuthValidator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -1640,7 +1596,7 @@ public async Task SetHandler_WithClientAppId_ShouldConfigureGraphApiService() _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockAuthValidator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -1674,7 +1630,7 @@ public async Task SetHandler_WithoutClientAppId_ShouldNotConfigureGraphApiServic _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockAuthValidator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -1708,7 +1664,7 @@ public async Task SetHandler_WithWhitespaceClientAppId_ShouldNotConfigureGraphAp _mockLogger, _mockConfigService, _mockExecutor, - _mockAzureValidator, + _mockAuthValidator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); 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..89c188c6 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,14 @@ public void BotConfigurator_DeleteEndpoint_ShouldBeCalledIndependentlyOfWebApp() AgentBlueprintId = "blueprint-id" }; var command = CleanupCommand.CreateCommand( - _mockLogger, - _mockConfigService, - _mockBotConfigurator, - _mockExecutor, + _mockLogger, + _mockConfigService, + _mockBotConfigurator, + _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, - _federatedCredentialService); + _federatedCredentialService, + _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..5436711b 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, _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, _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, _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, _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, _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, _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, _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, _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, _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, _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, _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, _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, _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, _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, _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, _mockAuthValidator); var args = new[] { "cleanup", "--config", "test.json" }; // Act @@ -632,7 +644,8 @@ public void CleanupCommand_ShouldAcceptConfirmationProviderParameter() _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, - _federatedCredentialService); + _federatedCredentialService, + _mockAuthValidator); command.Should().NotBeNull(); command.Name.Should().Be("cleanup"); @@ -645,7 +658,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, _mockAuthValidator); var blueprintCommand = command.Subcommands.First(sc => sc.Name == "blueprint"); // Assert @@ -666,7 +679,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, _mockAuthValidator); var args = new[] { "cleanup", "blueprint", "--endpoint-only", "--config", "test.json" }; // Simulate user confirmation with y @@ -725,7 +738,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, _mockAuthValidator); var args = new[] { "cleanup", "blueprint", "--endpoint-only", "--config", "test.json" }; // Act @@ -763,7 +776,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, _mockAuthValidator); var args = new[] { "cleanup", "blueprint", "--endpoint-only", "--config", "test.json" }; // Act @@ -803,7 +816,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, _mockAuthValidator); var args = new[] { "cleanup", "blueprint", "--endpoint-only", "--config", "test.json" }; var originalIn = Console.In; @@ -844,7 +857,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, _mockAuthValidator); var args = new[] { "cleanup", "blueprint", "--endpoint-only", "--config", "test.json" }; var originalIn = Console.In; @@ -898,7 +911,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, _mockAuthValidator); var args = new[] { "cleanup", "blueprint", "--endpoint-only", "--config", "test.json" }; // Act @@ -925,7 +938,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, _mockAuthValidator); var args = new[] { "cleanup", "blueprint", "--endpoint-only", "--config", "test.json" }; var originalIn = Console.In; @@ -964,7 +977,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, _mockAuthValidator); var args = new[] { "cleanup", "blueprint", "--endpoint-only", "--config", "test.json" }; var originalIn = Console.In; @@ -1003,7 +1016,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, _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/CopilotStudioSubcommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CopilotStudioSubcommandTests.cs index 7e37a903..34a1eec8 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CopilotStudioSubcommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CopilotStudioSubcommandTests.cs @@ -6,6 +6,7 @@ using Microsoft.Agents.A365.DevTools.Cli.Models; using Microsoft.Agents.A365.DevTools.Cli.Services; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using NSubstitute; using System.CommandLine; using Xunit; @@ -19,6 +20,7 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Commands; public class CopilotStudioSubcommandTests { private readonly ILogger _mockLogger; + private readonly AzureAuthValidator _mockAuthValidator; private readonly IConfigService _mockConfigService; private readonly CommandExecutor _mockExecutor; private readonly GraphApiService _mockGraphApiService; @@ -30,6 +32,7 @@ public CopilotStudioSubcommandTests() _mockConfigService = Substitute.For(); var mockExecutorLogger = Substitute.For>(); _mockExecutor = Substitute.ForPartsOf(mockExecutorLogger); + _mockAuthValidator = Substitute.ForPartsOf(NullLogger.Instance, _mockExecutor); _mockGraphApiService = Substitute.ForPartsOf(); _mockBlueprintService = Substitute.ForPartsOf(Substitute.For>(), _mockGraphApiService); } @@ -42,6 +45,7 @@ public void CreateCommand_ShouldHaveCorrectName() // Act var command = CopilotStudioSubcommand.CreateCommand( _mockLogger, + _mockAuthValidator, _mockConfigService, _mockExecutor, _mockGraphApiService, @@ -58,6 +62,7 @@ public void CreateCommand_ShouldHaveConfigOption() // Act var command = CopilotStudioSubcommand.CreateCommand( _mockLogger, + _mockAuthValidator, _mockConfigService, _mockExecutor, _mockGraphApiService, @@ -76,6 +81,7 @@ public void CreateCommand_ShouldHaveVerboseOption() // Act var command = CopilotStudioSubcommand.CreateCommand( _mockLogger, + _mockAuthValidator, _mockConfigService, _mockExecutor, _mockGraphApiService, @@ -94,6 +100,7 @@ public void CreateCommand_ShouldHaveDryRunOption() // Act var command = CopilotStudioSubcommand.CreateCommand( _mockLogger, + _mockAuthValidator, _mockConfigService, _mockExecutor, _mockGraphApiService, @@ -110,6 +117,7 @@ public void CreateCommand_Description_ShouldMentionPowerPlatformApi() // Act var command = CopilotStudioSubcommand.CreateCommand( _mockLogger, + _mockAuthValidator, _mockConfigService, _mockExecutor, _mockGraphApiService, @@ -129,6 +137,7 @@ public void CreateCommand_Description_ShouldMentionPrerequisites() // Act var command = CopilotStudioSubcommand.CreateCommand( _mockLogger, + _mockAuthValidator, _mockConfigService, _mockExecutor, _mockGraphApiService, 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..6f7376ce 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 @@ -5,10 +5,13 @@ using System.CommandLine.Builder; using System.CommandLine.IO; using System.CommandLine.Parsing; +using FluentAssertions; using Microsoft.Extensions.Logging; using Microsoft.Agents.A365.DevTools.Cli.Commands; +using Microsoft.Agents.A365.DevTools.Cli.Exceptions; 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 +26,7 @@ public class DeployCommandTests private readonly ConfigService _mockConfigService; private readonly CommandExecutor _mockExecutor; private readonly DeploymentService _mockDeploymentService; - private readonly IAzureValidator _mockAzureValidator; + private readonly AzureAuthValidator _mockAuthValidator; private readonly GraphApiService _mockGraphApiService; private readonly AgentBlueprintService _mockBlueprintService; @@ -52,7 +55,7 @@ public DeployCommandTests() mockNodeLogger, mockPythonLogger); - _mockAzureValidator = Substitute.For(); + _mockAuthValidator = Substitute.ForPartsOf(NullLogger.Instance, _mockExecutor); _mockGraphApiService = Substitute.ForPartsOf(Substitute.For>(), _mockExecutor); _mockBlueprintService = Substitute.ForPartsOf(Substitute.For>(), _mockGraphApiService); } @@ -66,7 +69,7 @@ public void UpdateCommand_Should_Not_Have_Atg_Subcommand() _mockConfigService, _mockExecutor, _mockDeploymentService, - _mockAzureValidator, + _mockAuthValidator, _mockGraphApiService, _mockBlueprintService); // Act @@ -85,7 +88,7 @@ public void UpdateCommand_Should_Have_Config_Option_With_Default() _mockConfigService, _mockExecutor, _mockDeploymentService, - _mockAzureValidator, + _mockAuthValidator, _mockGraphApiService, _mockBlueprintService); // Act @@ -105,7 +108,7 @@ public void UpdateCommand_Should_Have_Verbose_Option() _mockConfigService, _mockExecutor, _mockDeploymentService, - _mockAzureValidator, + _mockAuthValidator, _mockGraphApiService, _mockBlueprintService); // Act @@ -117,6 +120,48 @@ public void UpdateCommand_Should_Have_Verbose_Option() } + /// + /// Regression: HandleDeploymentException must not wrap a DeployAppException in another DeployAppException. + /// Wrapping caused the full az cli stderr (stored in the exception message) to be printed 3 times. + /// + [Fact] + public void HandleDeploymentException_WithDeployAppException_RethrowsWithoutWrapping() + { + // Arrange + var original = new DeployAppException("Site failed to start. Check runtime logs: https://myapp.scm.azurewebsites.net/api/logs/docker"); + var method = typeof(DeployCommand).GetMethod( + "HandleDeploymentException", + System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static); + + // Act + var act = () => method!.Invoke(null, new object[] { original, _mockLogger }); + + // Assert — must rethrow the same type without wrapping + act.Should().Throw() + .WithInnerException() + .Where(ex => ReferenceEquals(ex, original), "the same instance must be rethrown, not a new wrapper"); + } + + /// + /// Regression: HandleDeploymentException must wrap non-DeployAppException in DeployAppException. + /// + [Fact] + public void HandleDeploymentException_WithGenericException_WrapsInDeployAppException() + { + // Arrange + var original = new InvalidOperationException("Something unexpected"); + var method = typeof(DeployCommand).GetMethod( + "HandleDeploymentException", + System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static); + + // Act + var act = () => method!.Invoke(null, new object[] { original, _mockLogger }); + + // Assert — generic exceptions should be wrapped + act.Should().Throw() + .WithInnerException(); + } + // NOTE: Integration tests that verify actual service invocation through command execution // are omitted here as they require complex mocking of logging infrastructure. // The command functionality is tested through integration/end-to-end tests when running diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PermissionsSubcommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PermissionsSubcommandTests.cs index 0a4b53de..ab58dd7a 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PermissionsSubcommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PermissionsSubcommandTests.cs @@ -6,6 +6,7 @@ using Microsoft.Agents.A365.DevTools.Cli.Models; using Microsoft.Agents.A365.DevTools.Cli.Services; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using NSubstitute; using System.CommandLine; using Xunit; @@ -19,6 +20,7 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Commands; public class PermissionsSubcommandTests { private readonly ILogger _mockLogger; + private readonly AzureAuthValidator _mockAuthValidator; private readonly IConfigService _mockConfigService; private readonly CommandExecutor _mockExecutor; private readonly GraphApiService _mockGraphApiService; @@ -30,6 +32,7 @@ public PermissionsSubcommandTests() _mockConfigService = Substitute.For(); var mockExecutorLogger = Substitute.For>(); _mockExecutor = Substitute.ForPartsOf(mockExecutorLogger); + _mockAuthValidator = Substitute.ForPartsOf(NullLogger.Instance, _mockExecutor); _mockGraphApiService = Substitute.ForPartsOf(); _mockBlueprintService = Substitute.ForPartsOf(Substitute.For>(), _mockGraphApiService); } @@ -42,6 +45,7 @@ public void CreateCommand_ShouldHaveMcpSubcommand() // Act var command = PermissionsSubcommand.CreateCommand( _mockLogger, + _mockAuthValidator, _mockConfigService, _mockExecutor, _mockGraphApiService, _mockBlueprintService); @@ -57,6 +61,7 @@ public void CreateCommand_ShouldHaveBotSubcommand() // Act var command = PermissionsSubcommand.CreateCommand( _mockLogger, + _mockAuthValidator, _mockConfigService, _mockExecutor, _mockGraphApiService, _mockBlueprintService); @@ -72,6 +77,7 @@ public void CommandDescription_ShouldMentionRequiredPermissions() // Act var command = PermissionsSubcommand.CreateCommand( _mockLogger, + _mockAuthValidator, _mockConfigService, _mockExecutor, _mockGraphApiService, _mockBlueprintService); @@ -86,6 +92,7 @@ public void CreateCommand_ShouldHaveBothSubcommands() // Act var command = PermissionsSubcommand.CreateCommand( _mockLogger, + _mockAuthValidator, _mockConfigService, _mockExecutor, _mockGraphApiService, _mockBlueprintService); @@ -104,6 +111,7 @@ public void CreateCommand_ShouldBeUsableInCommandPipeline() // Act var command = PermissionsSubcommand.CreateCommand( _mockLogger, + _mockAuthValidator, _mockConfigService, _mockExecutor, _mockGraphApiService, _mockBlueprintService); @@ -124,6 +132,7 @@ public void McpSubcommand_ShouldHaveCorrectName() // Act var command = PermissionsSubcommand.CreateCommand( _mockLogger, + _mockAuthValidator, _mockConfigService, _mockExecutor, _mockGraphApiService, _mockBlueprintService); @@ -140,6 +149,7 @@ public void McpSubcommand_ShouldHaveConfigOption() // Act var command = PermissionsSubcommand.CreateCommand( _mockLogger, + _mockAuthValidator, _mockConfigService, _mockExecutor, _mockGraphApiService, _mockBlueprintService); @@ -159,6 +169,7 @@ public void McpSubcommand_ShouldHaveVerboseOption() // Act var command = PermissionsSubcommand.CreateCommand( _mockLogger, + _mockAuthValidator, _mockConfigService, _mockExecutor, _mockGraphApiService, _mockBlueprintService); @@ -178,6 +189,7 @@ public void McpSubcommand_ShouldHaveDryRunOption() // Act var command = PermissionsSubcommand.CreateCommand( _mockLogger, + _mockAuthValidator, _mockConfigService, _mockExecutor, _mockGraphApiService, _mockBlueprintService); @@ -196,6 +208,7 @@ public void McpSubcommand_DescriptionShouldBeInformativeAndActionable() // Act var command = PermissionsSubcommand.CreateCommand( _mockLogger, + _mockAuthValidator, _mockConfigService, _mockExecutor, _mockGraphApiService, _mockBlueprintService); @@ -217,6 +230,7 @@ public void BotSubcommand_ShouldHaveCorrectName() // Act var command = PermissionsSubcommand.CreateCommand( _mockLogger, + _mockAuthValidator, _mockConfigService, _mockExecutor, _mockGraphApiService, _mockBlueprintService); @@ -233,6 +247,7 @@ public void BotSubcommand_ShouldHaveConfigOption() // Act var command = PermissionsSubcommand.CreateCommand( _mockLogger, + _mockAuthValidator, _mockConfigService, _mockExecutor, _mockGraphApiService, _mockBlueprintService); @@ -252,6 +267,7 @@ public void BotSubcommand_ShouldHaveVerboseOption() // Act var command = PermissionsSubcommand.CreateCommand( _mockLogger, + _mockAuthValidator, _mockConfigService, _mockExecutor, _mockGraphApiService, _mockBlueprintService); @@ -271,6 +287,7 @@ public void BotSubcommand_ShouldHaveDryRunOption() // Act var command = PermissionsSubcommand.CreateCommand( _mockLogger, + _mockAuthValidator, _mockConfigService, _mockExecutor, _mockGraphApiService, _mockBlueprintService); @@ -289,6 +306,7 @@ public void BotSubcommand_DescriptionShouldMentionPrerequisites() // Act var command = PermissionsSubcommand.CreateCommand( _mockLogger, + _mockAuthValidator, _mockConfigService, _mockExecutor, _mockGraphApiService, _mockBlueprintService); @@ -305,6 +323,7 @@ public void BotSubcommand_DescriptionShouldBeInformativeAndActionable() // Act var command = PermissionsSubcommand.CreateCommand( _mockLogger, + _mockAuthValidator, _mockConfigService, _mockExecutor, _mockGraphApiService, _mockBlueprintService); @@ -501,6 +520,7 @@ public void BotSubcommand_Description_ShouldNotReferenceNonExistentEndpointComma // Act var command = PermissionsSubcommand.CreateCommand( _mockLogger, + _mockAuthValidator, _mockConfigService, _mockExecutor, _mockGraphApiService, _mockBlueprintService); @@ -521,6 +541,7 @@ public void BotSubcommand_Description_ShouldMentionPrerequisites() // Act var command = PermissionsSubcommand.CreateCommand( _mockLogger, + _mockAuthValidator, _mockConfigService, _mockExecutor, _mockGraphApiService, _mockBlueprintService); diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandTests.cs index e9fee923..e90adba4 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandTests.cs @@ -125,6 +125,7 @@ public async Task PublishCommand_WithDryRun_ShouldReturnExitCode0() AgentBlueprintId = "test-blueprint-id", AgentBlueprintDisplayName = "Test Agent", TenantId = "test-tenant", + ClientAppId = "test-client-app-id", DeploymentProjectPath = tempDir }; _configService.LoadAsync().Returns(config); 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..a1acbf48 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,32 +162,39 @@ 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); } [Fact] - public void GetRequirementChecks_IsUnionOfSystemAndConfigChecks() + public void GetRequirementChecks_SystemChecksRunBeforeConfigChecks() { - // GetRequirementChecks must exactly equal GetSystemRequirementChecks + GetConfigRequirementChecks. + // GetRequirementChecks returns system checks (FrontierPreview, PowerShellModules) + // before config checks (AzureAuth, Location, ClientApp). + var mockExecutor = Substitute.ForPartsOf(Substitute.For>()); + var mockAuthValidator = Substitute.ForPartsOf(NullLogger.Instance, mockExecutor); var mockValidator = Substitute.For(); - var all = RequirementsSubcommand.GetRequirementChecks(mockValidator); - var system = RequirementsSubcommand.GetSystemRequirementChecks(); - var config = RequirementsSubcommand.GetConfigRequirementChecks(mockValidator); + var all = RequirementsSubcommand.GetRequirementChecks(mockAuthValidator, mockValidator); - all.Should().HaveCount(system.Count + config.Count); - all.Select(c => c.GetType()).Should().StartWith(system.Select(c => c.GetType()), - "system checks run before config checks"); - all.Select(c => c.GetType()).Should().EndWith(config.Select(c => c.GetType()), - "config checks follow system checks"); + // System checks come first + var types = all.Select(c => c.GetType()).ToList(); + types.IndexOf(typeof(FrontierPreviewRequirementCheck)) + .Should().BeLessThan(types.IndexOf(typeof(AzureAuthRequirementCheck)), + "system checks should run before config checks"); + types.IndexOf(typeof(PowerShellModulesRequirementCheck)) + .Should().BeLessThan(types.IndexOf(typeof(LocationRequirementCheck)), + "system checks should run before config checks"); } #endregion 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 a5c86135..7759d63c 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,7 @@ public class SetupCommandTests private readonly CommandExecutor _mockExecutor; private readonly DeploymentService _mockDeploymentService; private readonly IBotConfigurator _mockBotConfigurator; - private readonly IAzureValidator _mockAzureValidator; + private readonly AzureAuthValidator _mockAuthValidator; private readonly PlatformDetector _mockPlatformDetector; private readonly GraphApiService _mockGraphApiService; private readonly AgentBlueprintService _mockBlueprintService; @@ -52,7 +53,7 @@ public SetupCommandTests() mockNodeLogger, mockPythonLogger); _mockBotConfigurator = Substitute.For(); - _mockAzureValidator = Substitute.For(); + _mockAuthValidator = Substitute.ForPartsOf(NullLogger.Instance, _mockExecutor); _mockGraphApiService = Substitute.For(); _mockBlueprintService = Substitute.ForPartsOf(Substitute.For>(), _mockGraphApiService); _mockClientAppValidator = Substitute.For(); @@ -79,15 +80,15 @@ public async Task SetupAllCommand_DryRun_ValidConfig_OnlyValidatesConfig() _mockConfigService.LoadAsync(Arg.Any(), Arg.Any()).Returns(Task.FromResult(config)); var command = SetupCommand.CreateCommand( - _mockLogger, - _mockConfigService, - _mockExecutor, - _mockDeploymentService, - _mockBotConfigurator, - _mockAzureValidator, + _mockLogger, + _mockConfigService, + _mockExecutor, + _mockDeploymentService, + _mockBotConfigurator, + _mockAuthValidator, _mockPlatformDetector, _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService, _mockClientAppValidator); - + var parser = new CommandLineBuilder(command).Build(); var testConsole = new TestConsole(); @@ -99,7 +100,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!); } @@ -129,8 +129,8 @@ public async Task SetupAllCommand_SkipInfrastructure_SkipsInfrastructureStep() _mockConfigService, _mockExecutor, _mockDeploymentService, - _mockBotConfigurator, - _mockAzureValidator, + _mockBotConfigurator, + _mockAuthValidator, _mockPlatformDetector, _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService, _mockClientAppValidator); @@ -156,8 +156,8 @@ public void SetupCommand_HasRequiredSubcommands() _mockConfigService, _mockExecutor, _mockDeploymentService, - _mockBotConfigurator, - _mockAzureValidator, + _mockBotConfigurator, + _mockAuthValidator, _mockPlatformDetector, _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService, _mockClientAppValidator); @@ -180,8 +180,8 @@ public void SetupCommand_PermissionsSubcommand_HasMcpAndBotSubcommands() _mockConfigService, _mockExecutor, _mockDeploymentService, - _mockBotConfigurator, - _mockAzureValidator, + _mockBotConfigurator, + _mockAuthValidator, _mockPlatformDetector, _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService, _mockClientAppValidator); @@ -207,8 +207,8 @@ public void SetupCommand_ErrorMessages_ShouldBeInformativeAndActionable() _mockConfigService, _mockExecutor, _mockDeploymentService, - _mockBotConfigurator, - _mockAzureValidator, + _mockBotConfigurator, + _mockAuthValidator, _mockPlatformDetector, _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService, _mockClientAppValidator); @@ -252,8 +252,8 @@ public async Task InfrastructureSubcommand_DryRun_CompletesSuccessfully() _mockConfigService, _mockExecutor, _mockDeploymentService, - _mockBotConfigurator, - _mockAzureValidator, + _mockBotConfigurator, + _mockAuthValidator, _mockPlatformDetector, _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService, _mockClientAppValidator); @@ -295,7 +295,7 @@ public async Task BlueprintSubcommand_DryRun_CompletesSuccessfully() _mockExecutor, _mockDeploymentService, _mockBotConfigurator, - _mockAzureValidator, + _mockAuthValidator, _mockPlatformDetector, _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService, _mockClientAppValidator); @@ -336,7 +336,7 @@ public async Task RequirementsSubcommand_ValidConfig_CompletesSuccessfully() _mockExecutor, _mockDeploymentService, _mockBotConfigurator, - _mockAzureValidator, + _mockAuthValidator, _mockPlatformDetector, _mockGraphApiService, _mockBlueprintService, @@ -379,7 +379,7 @@ public async Task RequirementsSubcommand_WithCategoryFilter_RunsFilteredChecks() _mockExecutor, _mockDeploymentService, _mockBotConfigurator, - _mockAzureValidator, + _mockAuthValidator, _mockPlatformDetector, _mockGraphApiService, _mockBlueprintService, 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/ClientAppValidatorTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/ClientAppValidatorTests.cs index 20a4f51c..c54bd6e6 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/ClientAppValidatorTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/ClientAppValidatorTests.cs @@ -271,6 +271,7 @@ public async Task EnsureValidClientAppAsync_WhenAppNotFound_ThrowsClientAppValid _executor.ExecuteAsync( Arg.Is(s => s == "az"), Arg.Is(s => s.Contains("rest --method GET") && s.Contains("/applications")), + suppressErrorLogging: Arg.Any(), cancellationToken: Arg.Any()) .Returns(new CommandResult { ExitCode = 0, StandardOutput = "{\"value\": []}", StandardError = string.Empty }); @@ -324,6 +325,7 @@ private void SetupTokenAcquisition(string token) _executor.ExecuteAsync( Arg.Is(s => s == "az"), Arg.Is(s => s.Contains("account get-access-token")), + suppressErrorLogging: Arg.Any(), cancellationToken: Arg.Any()) .Returns(new CommandResult { ExitCode = 0, StandardOutput = token, StandardError = string.Empty }); } @@ -347,6 +349,7 @@ private void SetupAppExists(string appId, string displayName, string? requiredRe _executor.ExecuteAsync( Arg.Is(s => s == "az"), Arg.Is(s => s.Contains("rest --method GET") && s.Contains("/applications")), + suppressErrorLogging: Arg.Any(), cancellationToken: Arg.Any()) .Returns(new CommandResult { ExitCode = 0, StandardOutput = appJson, StandardError = string.Empty }); } diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/DeploymentServiceTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/DeploymentServiceTests.cs index 20d9e82a..f980813b 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/DeploymentServiceTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/DeploymentServiceTests.cs @@ -3,6 +3,7 @@ using FluentAssertions; using Microsoft.Agents.A365.DevTools.Cli.Commands.SetupSubcommands; +using Microsoft.Agents.A365.DevTools.Cli.Exceptions; using Microsoft.Agents.A365.DevTools.Cli.Models; using Microsoft.Agents.A365.DevTools.Cli.Services; using Microsoft.Extensions.Logging; @@ -153,4 +154,109 @@ public async Task GetLinuxFxVersionForPlatformAsync_Unknown_DefaultsToDotNet8() // Assert result.Should().Be("DOTNETCORE|8.0", "Unknown platform should default to .NET 8.0 to avoid PHP container selection"); } -} + + /// + /// Regression: DeployAppException message must not contain raw az cli stderr. + /// The site-start timeout case should produce a short, actionable message with the docker logs URL. + /// Uses restart=true to bypass the build pipeline and test only the Azure deploy error path. + /// + [Theory] + [InlineData("ERROR: Deployment failed because the site failed to start within 10 mins.\nInprogressInstances: 0")] + [InlineData("Error: Deployment for site 'myapp' failed because the worker proccess failed to start within the allotted time.")] + public async Task DeployAsync_SiteStartTimeout_ThrowsDeployAppExceptionWithDockerLogUrl(string azCliStderr) + { + // Arrange — create a temporary publish directory so restart=true path succeeds past the dir check + var publishDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString(), "publish"); + Directory.CreateDirectory(publishDir); + File.WriteAllText(Path.Combine(publishDir, "index.js"), "// test"); + + try + { + var projectDir = Path.GetDirectoryName(publishDir)!; + var config = new DeploymentConfiguration + { + ResourceGroup = "rg-test", + AppName = "myapp", + ProjectPath = projectDir, + DeploymentZip = "app.zip", + PublishOutputPath = "publish", + Platform = ProjectPlatform.NodeJs + }; + + _mockExecutor + .ExecuteWithStreamingAsync( + Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any(), Arg.Any(), Arg.Any>(), + Arg.Any(), Arg.Any()) + .Returns(new CommandResult { ExitCode = 1, StandardError = azCliStderr }); + + _mockExecutor + .ExecuteAsync(Arg.Any(), Arg.Any(), + Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(new CommandResult { ExitCode = 0 }); + + // Act — restart=true skips build pipeline, goes directly to zip + deploy + var act = async () => await _deploymentService.DeployAsync(config, verbose: false, restart: true); + + // Assert + var ex = await act.Should().ThrowAsync(); + ex.Which.Message.Should().Contain("scm.azurewebsites.net", "docker log URL must be in the exception message"); + ex.Which.Message.Should().NotContain("WARNING:", "raw az cli stderr must not leak into the exception message"); + ex.Which.Message.Should().NotContain("InprogressInstances:", "raw az cli polling output must not leak into the exception message"); + } + finally + { + Directory.Delete(Path.GetDirectoryName(publishDir)!, recursive: true); + } + } + + /// + /// Regression: a generic az cli failure (not timeout) should produce a short message with exit code only. + /// + [Fact] + public async Task DeployAsync_GenericAzCliFailure_ThrowsDeployAppExceptionWithExitCodeOnly() + { + // Arrange + var publishDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString(), "publish"); + Directory.CreateDirectory(publishDir); + File.WriteAllText(Path.Combine(publishDir, "index.js"), "// test"); + + try + { + var projectDir = Path.GetDirectoryName(publishDir)!; + var config = new DeploymentConfiguration + { + ResourceGroup = "rg-test", + AppName = "myapp", + ProjectPath = projectDir, + DeploymentZip = "app.zip", + PublishOutputPath = "publish", + Platform = ProjectPlatform.NodeJs + }; + + _mockExecutor + .ExecuteWithStreamingAsync( + Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any(), Arg.Any(), Arg.Any>(), + Arg.Any(), Arg.Any()) + .Returns(new CommandResult { ExitCode = 2, StandardError = "WARNING: Some az output\nERROR: Resource group not found" }); + + _mockExecutor + .ExecuteAsync(Arg.Any(), Arg.Any(), + Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(new CommandResult { ExitCode = 0 }); + + // Act + var act = async () => await _deploymentService.DeployAsync(config, verbose: false, restart: true); + + // Assert + var ex = await act.Should().ThrowAsync(); + ex.Which.Message.Should().Contain("exit code 2"); + ex.Which.Message.Should().NotContain("WARNING:", "raw az cli stderr must not appear in the exception message"); + } + finally + { + Directory.Delete(Path.GetDirectoryName(publishDir)!, recursive: true); + } + } +} diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Helpers/CleanConsoleFormatterTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Helpers/CleanConsoleFormatterTests.cs index ecb3fcec..0f111494 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Helpers/CleanConsoleFormatterTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Helpers/CleanConsoleFormatterTests.cs @@ -88,7 +88,7 @@ public void Write_WithCriticalLevel_OutputsMessageWithErrorPrefix() } [Fact] - public void Write_WithWarningLevel_OutputsMessageWithWarningPrefix() + public void Write_WithWarningLevel_OutputsMessageWithoutWarningPrefix() { // Arrange var message = "This is a warning message"; @@ -97,9 +97,9 @@ public void Write_WithWarningLevel_OutputsMessageWithWarningPrefix() // Act _formatter.Write(logEntry, null, _consoleWriter); - // Assert + // Assert - warning messages are yellow but have no "WARNING:" prefix (message already contains [WARN] tag) var output = _consoleWriter.ToString(); - output.Should().Contain("WARNING:"); + output.Should().NotContain("WARNING:"); output.Should().Contain(message); } @@ -135,7 +135,7 @@ public void Write_WithExceptionAndWarning_IncludesExceptionDetails() // Assert var output = _consoleWriter.ToString(); - output.Should().Contain("WARNING:"); + output.Should().NotContain("WARNING:"); output.Should().Contain(message); output.Should().Contain("Test warning exception"); output.Should().Contain("ArgumentException"); @@ -229,6 +229,7 @@ public void Constructor_CreatesFormatterWithCleanName() [Theory] [InlineData(LogLevel.Information)] + [InlineData(LogLevel.Warning)] [InlineData(LogLevel.Debug)] [InlineData(LogLevel.Trace)] public void Write_WithNonWarningOrErrorLevel_DoesNotIncludePrefix(LogLevel logLevel) diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/MicrosoftGraphTokenProviderTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/MicrosoftGraphTokenProviderTests.cs index 17200c86..18a05ccd 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/MicrosoftGraphTokenProviderTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/MicrosoftGraphTokenProviderTests.cs @@ -36,6 +36,7 @@ public async Task GetMgGraphAccessTokenAsync_WithValidClientAppId_IncludesClient Arg.Any(), Arg.Any(), Arg.Any?>(), + Arg.Any(), Arg.Any()) .Returns(new CommandResult { ExitCode = 0, StandardOutput = expectedToken, StandardError = string.Empty }); @@ -53,6 +54,7 @@ await _executor.Received(1).ExecuteWithStreamingAsync( Arg.Any(), Arg.Any(), Arg.Any?>(), + Arg.Any(), Arg.Any()); } @@ -71,6 +73,7 @@ public async Task GetMgGraphAccessTokenAsync_WithoutClientAppId_OmitsClientIdPar Arg.Any(), Arg.Any(), Arg.Any?>(), + Arg.Any(), Arg.Any()) .Returns(new CommandResult { ExitCode = 0, StandardOutput = expectedToken, StandardError = string.Empty }); @@ -88,6 +91,7 @@ await _executor.Received(1).ExecuteWithStreamingAsync( Arg.Any(), Arg.Any(), Arg.Any?>(), + Arg.Any(), Arg.Any()); } @@ -163,6 +167,7 @@ public async Task GetMgGraphAccessTokenAsync_WhenExecutionFails_ReturnsNull() Arg.Any(), Arg.Any(), Arg.Any?>(), + Arg.Any(), Arg.Any()) .Returns(new CommandResult { ExitCode = 1, StandardOutput = string.Empty, StandardError = "PowerShell error" }); @@ -190,6 +195,7 @@ public async Task GetMgGraphAccessTokenAsync_WithValidToken_ReturnsToken() Arg.Any(), Arg.Any(), Arg.Any?>(), + Arg.Any(), Arg.Any()) .Returns(new CommandResult { ExitCode = 0, StandardOutput = expectedToken, StandardError = string.Empty }); @@ -236,6 +242,7 @@ public async Task GetMgGraphAccessTokenAsync_EscapesSingleQuotesInClientAppId() Arg.Any(), Arg.Any(), Arg.Any?>(), + Arg.Any(), Arg.Any()) .Returns(new CommandResult { ExitCode = 0, StandardOutput = expectedToken, StandardError = string.Empty }); 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..ad1ccbbb --- /dev/null +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/PrerequisiteRunnerTests.cs @@ -0,0 +1,162 @@ +// 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_ShouldReturnTrue() + { + // 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: warnings do not block execution. + // Warning output is emitted by the check itself (via ExecuteCheckWithLoggingAsync), + // not by the runner. + result.Should().BeTrue("a warning does not block execution"); + } + + [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/AppServiceAuthRequirementCheckTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Requirements/AppServiceAuthRequirementCheckTests.cs new file mode 100644 index 00000000..445f3bd9 --- /dev/null +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Requirements/AppServiceAuthRequirementCheckTests.cs @@ -0,0 +1,108 @@ +// 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 AppServiceAuthRequirementCheck +/// +public class AppServiceAuthRequirementCheckTests +{ + private readonly AzureAuthValidator _mockAuthValidator; + private readonly ILogger _mockLogger; + + public AppServiceAuthRequirementCheckTests() + { + var mockExecutor = Substitute.ForPartsOf(NullLogger.Instance); + _mockAuthValidator = Substitute.ForPartsOf(NullLogger.Instance, mockExecutor); + _mockLogger = Substitute.For(); + } + + [Fact] + public async Task CheckAsync_WhenTokenAcquisitionSucceeds_ShouldReturnSuccess() + { + // Arrange + var check = new AppServiceAuthRequirementCheck(_mockAuthValidator); + var config = new Agent365Config { SubscriptionId = "test-sub-id" }; + + _mockAuthValidator.GetAppServiceTokenAsync(Arg.Any()) + .Returns(true); + + // Act + var result = await check.CheckAsync(config, _mockLogger); + + // Assert + result.Should().NotBeNull(); + result.Passed.Should().BeTrue(); + result.IsWarning.Should().BeFalse(); + result.ErrorMessage.Should().BeNullOrEmpty(); + } + + [Fact] + public async Task CheckAsync_WhenTokenAcquisitionFails_ShouldReturnFailure() + { + // Arrange + var check = new AppServiceAuthRequirementCheck(_mockAuthValidator); + var config = new Agent365Config { SubscriptionId = "test-sub-id" }; + + _mockAuthValidator.GetAppServiceTokenAsync(Arg.Any()) + .Returns(false); + + // Act + var result = await check.CheckAsync(config, _mockLogger); + + // Assert + result.Should().NotBeNull(); + result.Passed.Should().BeFalse(); + result.ErrorMessage.Should().Contain("App Service token is expired or revoked"); + result.ResolutionGuidance.Should().Contain("az logout"); + } + + [Fact] + public async Task CheckAsync_ShouldCallGetAppServiceTokenAsync() + { + // Arrange + var check = new AppServiceAuthRequirementCheck(_mockAuthValidator); + var config = new Agent365Config(); + + _mockAuthValidator.GetAppServiceTokenAsync(Arg.Any()) + .Returns(true); + + // Act + await check.CheckAsync(config, _mockLogger); + + // Assert + await _mockAuthValidator.Received(1).GetAppServiceTokenAsync(Arg.Any()); + } + + [Fact] + public void Metadata_ShouldHaveCorrectName() + { + var check = new AppServiceAuthRequirementCheck(_mockAuthValidator); + check.Name.Should().Be("App Service Authentication"); + } + + [Fact] + public void Metadata_ShouldHaveCorrectCategory() + { + var check = new AppServiceAuthRequirementCheck(_mockAuthValidator); + check.Category.Should().Be("Azure"); + } + + [Fact] + public void Constructor_WithNullValidator_ShouldThrowArgumentNullException() + { + var act = () => new AppServiceAuthRequirementCheck(null!); + act.Should().Throw() + .WithParameterName("auth"); + } +} 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/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..c4fa69c2 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"); } @@ -53,12 +52,11 @@ public async Task CheckAsync_ShouldLogMainWarningMessage() // Act await check.CheckAsync(config, _mockLogger); - // Assert - // Verify the logger was called with the main warning message + // Assert — [WARN] output is logged at Warning severity (yellow color, no WARNING: text prefix from formatter) _mockLogger.Received().Log( LogLevel.Warning, 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>()); } @@ -73,12 +71,11 @@ public async Task CheckAsync_ShouldLogRequirementName() // Act await check.CheckAsync(config, _mockLogger); - // Assert - // Verify the logger was called with "Requirement:" prefix + // Assert — warning is logged at Warning severity and includes the check name _mockLogger.Received().Log( - LogLevel.Information, + LogLevel.Warning, 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 +91,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 +106,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] 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"); + } +} diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Requirements/MosPrerequisitesRequirementCheckTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Requirements/MosPrerequisitesRequirementCheckTests.cs new file mode 100644 index 00000000..5b04c300 --- /dev/null +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Requirements/MosPrerequisitesRequirementCheckTests.cs @@ -0,0 +1,128 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using FluentAssertions; +using Microsoft.Agents.A365.DevTools.Cli.Exceptions; +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 MosPrerequisitesRequirementCheck +/// +public class MosPrerequisitesRequirementCheckTests +{ + private readonly GraphApiService _mockGraphApiService; + private readonly AgentBlueprintService _mockBlueprintService; + private readonly ILogger _mockLogger; + + public MosPrerequisitesRequirementCheckTests() + { + var mockExecutor = Substitute.ForPartsOf(NullLogger.Instance); + _mockGraphApiService = Substitute.For(NullLogger.Instance, mockExecutor); + _mockBlueprintService = Substitute.ForPartsOf(NullLogger.Instance, _mockGraphApiService); + _mockLogger = Substitute.For(); + } + + [Fact] + public async Task CheckAsync_WhenClientAppIdMissing_ShouldReturnFailure() + { + // Arrange — missing ClientAppId causes EnsureMosPrerequisitesAsync to throw SetupValidationException + var check = new MosPrerequisitesRequirementCheck(_mockGraphApiService, _mockBlueprintService); + var config = new Agent365Config { TenantId = "test-tenant" }; // no ClientAppId + + // Act + var result = await check.CheckAsync(config, _mockLogger); + + // Assert + result.Should().NotBeNull(); + result.Passed.Should().BeFalse(); + result.ErrorMessage.Should().NotBeNullOrEmpty(); + } + + [Fact] + public async Task CheckAsync_WhenSetupValidationExceptionHasMitigationSteps_ShouldIncludeThemInResolution() + { + // Arrange — mock GraphGetAsync to throw SetupValidationException with explicit mitigation steps + var mitigationStep = "Grant admin consent via https://entra.microsoft.com"; + var check = new MosPrerequisitesRequirementCheck(_mockGraphApiService, _mockBlueprintService); + var config = new Agent365Config + { + TenantId = "test-tenant", + ClientAppId = "00000000-0000-0000-0000-000000000001" + }; + + _mockGraphApiService.GraphGetAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any?>()) + .Returns(Task.FromException(new SetupValidationException( + issueDescription: "MOS service principal not found", + mitigationSteps: [mitigationStep]))); + + // Act + var result = await check.CheckAsync(config, _mockLogger); + + // Assert — mitigation steps from the exception must appear in ResolutionGuidance + result.Passed.Should().BeFalse(); + result.ResolutionGuidance.Should().Contain(mitigationStep); + } + + [Fact] + public async Task CheckAsync_WhenSetupValidationExceptionHasNoMitigationSteps_ShouldUseFallbackResolution() + { + // Arrange — GraphGetAsync returns null for the app lookup, causing SetupValidationException + // with no mitigation steps (the default exception message is used) + var check = new MosPrerequisitesRequirementCheck(_mockGraphApiService, _mockBlueprintService); + var config = new Agent365Config + { + TenantId = "test-tenant", + ClientAppId = "00000000-0000-0000-0000-000000000001" + }; + + _mockGraphApiService.GraphGetAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any?>()) + .Returns((System.Text.Json.JsonDocument?)null); + + // Act + var result = await check.CheckAsync(config, _mockLogger); + + // Assert — app not found throws SetupValidationException, check maps it to Failure with fallback guidance + result.Passed.Should().BeFalse(); + result.ResolutionGuidance.Should().Contain("a365 setup all"); + } + + [Fact] + public void Metadata_ShouldHaveCorrectName() + { + var check = new MosPrerequisitesRequirementCheck(_mockGraphApiService, _mockBlueprintService); + check.Name.Should().Be("MOS Prerequisites"); + } + + [Fact] + public void Metadata_ShouldHaveCorrectCategory() + { + var check = new MosPrerequisitesRequirementCheck(_mockGraphApiService, _mockBlueprintService); + check.Category.Should().Be("MOS"); + } + + [Fact] + public void Constructor_WithNullGraphApiService_ShouldThrowArgumentNullException() + { + var act = () => new MosPrerequisitesRequirementCheck(null!, _mockBlueprintService); + act.Should().Throw() + .WithParameterName("graphApiService"); + } + + [Fact] + public void Constructor_WithNullBlueprintService_ShouldThrowArgumentNullException() + { + var act = () => new MosPrerequisitesRequirementCheck(_mockGraphApiService, null!); + act.Should().Throw() + .WithParameterName("blueprintService"); + } +}