From 85d0c854f807e8aeb27cb2b5a679f28f2b09c125 Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Tue, 10 Mar 2026 18:05:10 -0700 Subject: [PATCH 1/6] fix: strip publish command to zip-only and remove internal MOS constants - PublishCommand now only updates manifest files, creates manifest.zip, and prints manual upload instructions (Agents > All agents > Upload custom agent) - Removed MOS Titles upload, token acquisition, and Graph API steps from publish - Removed --mos-env, --mos-token, --skip-graph options - Deleted AgentPublishService, MosTokenService, PublishHelpers (dead code) - Stripped MosConstants to PowerPlatform-only; removed internal TPS/MOS Titles app IDs, scope GUIDs, and service URLs - Removed dead MOS error message helpers from ErrorMessages - Updated PublishCommandTests to match new simplified signature and added zip creation test Co-Authored-By: Claude Sonnet 4.6 --- .../Commands/PublishCommand.cs | 462 +------------- .../Constants/ErrorMessages.cs | 43 -- .../Constants/MosConstants.cs | 193 +----- .../Helpers/PublishHelpers.cs | 590 ------------------ .../Program.cs | 4 +- .../Services/AgentPublishService.cs | 343 ---------- .../Services/MosTokenService.cs | 262 -------- .../Commands/PublishCommandTests.cs | 87 ++- .../Commands/PublishHelpersTests.cs | 465 -------------- .../Services/AgentPublishServiceTests.cs | 231 ------- .../Services/MosTokenServiceCacheTests.cs | 213 ------- .../Services/MosTokenServiceTests.cs | 171 ----- 12 files changed, 89 insertions(+), 2975 deletions(-) delete mode 100644 src/Microsoft.Agents.A365.DevTools.Cli/Helpers/PublishHelpers.cs delete mode 100644 src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentPublishService.cs delete mode 100644 src/Microsoft.Agents.A365.DevTools.Cli/Services/MosTokenService.cs delete mode 100644 src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishHelpersTests.cs delete mode 100644 src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentPublishServiceTests.cs delete mode 100644 src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/MosTokenServiceCacheTests.cs delete mode 100644 src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/MosTokenServiceTests.cs diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommand.cs index fce13d0a..9f2c93cb 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommand.cs @@ -1,50 +1,23 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -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.Internal; using Microsoft.Extensions.Logging; -using Microsoft.Identity.Client; using System.CommandLine; using System.IO.Compression; -using System.Net.Http.Headers; using System.Text.Json; using System.Text.Json.Nodes; namespace Microsoft.Agents.A365.DevTools.Cli.Commands; /// -/// Publish command – updates manifest.json ids based on the generated agent blueprint id. -/// Native C# implementation - no PowerShell dependencies. +/// Publish command – updates manifest.json ids based on the generated agent blueprint id +/// and packages the manifest files into a zip ready for manual upload. /// public class PublishCommand { - // MOS Titles service URLs - private const string MosTitlesUrlProd = "https://titles.prod.mos.microsoft.com"; - - /// - /// Gets the appropriate MOS Titles URL based on environment variable override or defaults to production. - /// Set MOS_TITLES_URL environment variable to override the default production URL. - /// - /// Tenant ID (not used, kept for backward compatibility) - /// MOS Titles base URL from environment variable or production default - private static string GetMosTitlesUrl(string? tenantId) - { - // Check for environment variable override - var envUrl = Environment.GetEnvironmentVariable("MOS_TITLES_URL"); - if (!string.IsNullOrWhiteSpace(envUrl)) - { - return envUrl; - } - - return MosTitlesUrlProd; - } - /// /// Gets the project directory from config, with fallback to current directory. /// Ensures absolute path resolution for portability. @@ -87,51 +60,31 @@ private static string GetProjectDirectory(Agent365Config config, ILogger logger) public static Command CreateCommand( ILogger logger, IConfigService configService, - AgentPublishService agentPublishService, - GraphApiService graphApiService, - AgentBlueprintService blueprintService, + AgentBlueprintService agentBlueprintService, ManifestTemplateService manifestTemplateService) { - var command = new Command("publish", "Update manifest.json IDs and publish package; configure federated identity and app role assignments"); + var command = new Command("publish", "Update manifest.json IDs and create a manifest package for upload to the Microsoft 365 Admin Center"); var dryRunOption = new Option("--dry-run", "Show changes without writing file or calling APIs"); - var skipGraphOption = new Option("--skip-graph", "Skip Graph federated identity and role assignment steps"); - var mosEnvOption = new Option("--mos-env", () => "prod", "MOS environment identifier (e.g. prod, dev) - use MOS_TITLES_URL environment variable for custom URLs"); - var mosPersonalTokenOption = new Option("--mos-token", () => Environment.GetEnvironmentVariable("MOS_PERSONAL_TOKEN"), "Override MOS token (personal token) - bypass script & cache"); var verboseOption = new Option( ["--verbose", "-v"], description: "Enable verbose logging"); - + command.AddOption(dryRunOption); - command.AddOption(skipGraphOption); - command.AddOption(mosEnvOption); - command.AddOption(mosPersonalTokenOption); command.AddOption(verboseOption); command.SetHandler(async (System.CommandLine.Invocation.InvocationContext context) => { - // Extract options from invocation context (enables context.ExitCode on error paths) var dryRun = context.ParseResult.GetValueForOption(dryRunOption); - var skipGraph = context.ParseResult.GetValueForOption(skipGraphOption); - var mosEnv = context.ParseResult.GetValueForOption(mosEnvOption) ?? "prod"; - var mosPersonalToken = context.ParseResult.GetValueForOption(mosPersonalTokenOption); - var verbose = context.ParseResult.GetValueForOption(verboseOption); - // Track whether the command completed normally (success or expected early exit) - // All unhandled error paths will set context.ExitCode = 1 var isNormalExit = false; - // Generate correlation ID at workflow entry point - var correlationId = HttpClientFactory.GenerateCorrelationId(); - try { // Load configuration using ConfigService var config = await configService.LoadAsync(); - logger.LogDebug("Configuration loaded successfully (CorrelationId: {CorrelationId})", correlationId); // Extract required values from config - var tenantId = config.TenantId; var agentBlueprintDisplayName = config.AgentBlueprintDisplayName; var blueprintId = config.AgentBlueprintId; @@ -187,16 +140,6 @@ public static Command CreateCommand( return; } - // Determine MOS Titles URL based on tenant - 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."); - } - string updatedManifest = await UpdateManifestFileAsync(logger, agentBlueprintDisplayName, blueprintId, manifestPath); string updatedAgenticUserManifestTemplate = await UpdateAgenticUserManifestTemplateFileAsync(logger, agentBlueprintDisplayName, blueprintId, agenticUserManifestTemplatePath); @@ -205,7 +148,7 @@ public static Command CreateCommand( { logger.LogInformation("DRY RUN: Updated manifest (not saved):\n{Json}", updatedManifest); logger.LogInformation("DRY RUN: Updated agentic user manifest template (not saved):\n{Json}", updatedAgenticUserManifestTemplate); - logger.LogInformation("DRY RUN: Skipping zipping & API calls"); + logger.LogInformation("DRY RUN: Skipping zipping"); isNormalExit = true; return; } @@ -247,7 +190,7 @@ public static Command CreateCommand( logger.LogInformation(" Icons"); logger.LogInformation(" - Replace 'color.png' and 'outline.png' with your custom branding"); logger.LogInformation(""); - + // Ask if user wants to open the file now (skip when stdin is not a terminal) if (!Console.IsInputRedirected) { @@ -259,15 +202,15 @@ public static Command CreateCommand( FileHelper.TryOpenFileInDefaultEditor(manifestPath, logger); } - Console.Write("Press Enter when you have finished editing the manifest to continue with publish: "); + Console.Write("Press Enter when you have finished editing the manifest to continue: "); Console.Out.Flush(); Console.ReadLine(); } - logger.LogInformation("Continuing with publish process..."); + logger.LogInformation("Creating manifest package..."); logger.LogInformation(""); - // Step 1: Create manifest.zip including the four required files + // Create manifest.zip including the required files var zipPath = Path.Combine(manifestDir, "manifest.zip"); if (File.Exists(zipPath)) { @@ -315,372 +258,22 @@ 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(); - var mosTokenService = new MosTokenService( - cleanLoggerFactory.CreateLogger(), - configService); - - string? mosToken = null; - try - { - mosToken = await mosTokenService.AcquireTokenAsync(mosEnv, mosPersonalToken); - logger.LogDebug("MOS token acquired successfully"); - } - catch (MsalServiceException ex) when (ex.ErrorCode == "invalid_client" && - ex.Message.Contains("AADSTS650052")) - { - logger.LogError("MOS token acquisition failed: Missing service principal or admin consent (Error: {ErrorCode})", ex.ErrorCode); - logger.LogInformation(""); - logger.LogInformation("The MOS service principals exist, but admin consent may not be granted."); - logger.LogInformation("Grant admin consent at:"); - logger.LogInformation(" {PortalUrl}", - MosConstants.GetApiPermissionsPortalUrl(config.ClientAppId)); - logger.LogInformation(""); - logger.LogInformation("Or authenticate interactively and consent when prompted."); - logger.LogInformation(""); - return; - } - catch (MsalServiceException ex) when (ex.ErrorCode == "unauthorized_client" && - ex.Message.Contains("AADSTS50194")) - { - logger.LogError("MOS token acquisition failed: Single-tenant app cannot use /common endpoint (Error: {ErrorCode})", ex.ErrorCode); - logger.LogInformation(""); - logger.LogInformation("AADSTS50194: The application is configured as single-tenant but is trying to use the /common authority."); - logger.LogInformation("This should be automatically handled by using tenant-specific authority URLs."); - logger.LogInformation(""); - logger.LogInformation("If this error persists:"); - logger.LogInformation("1. Verify your app registration is configured correctly in Azure Portal"); - logger.LogInformation("2. Check that tenantId in a365.config.json matches your app's home tenant"); - logger.LogInformation("3. Ensure the app's 'Supported account types' setting matches your use case"); - logger.LogInformation(""); - return; - } - catch (MsalServiceException ex) when (ex.ErrorCode == "invalid_grant") - { - logger.LogError("MOS token acquisition failed: Invalid or expired credentials (Error: {ErrorCode})", ex.ErrorCode); - logger.LogInformation(""); - logger.LogInformation("The authentication failed due to invalid credentials or expired tokens."); - logger.LogInformation("Try clearing the token cache and re-authenticating:"); - logger.LogInformation(" - Delete: ~/.a365/mos-token-cache.json"); - logger.LogInformation(" - Run: a365 publish"); - logger.LogInformation(""); - return; - } - catch (MsalServiceException ex) - { - // Log all MSAL-specific errors with full context for debugging - logger.LogError("MOS token acquisition failed with MSAL error"); - logger.LogError("Error Code: {ErrorCode}", ex.ErrorCode); - logger.LogError("Error Message: {Message}", ex.Message); - logger.LogDebug("Stack Trace: {StackTrace}", ex.StackTrace); - - logger.LogInformation(""); - logger.LogInformation("Authentication failed. Common issues:"); - logger.LogInformation("1. Missing admin consent - Grant at:"); - logger.LogInformation(" {PortalUrl}", - MosConstants.GetApiPermissionsPortalUrl(config.ClientAppId)); - logger.LogInformation("2. Insufficient permissions - Verify required API permissions are configured"); - logger.LogInformation("3. Tenant configuration - Ensure app registration matches your tenant setup"); - logger.LogInformation(""); - logger.LogInformation("For detailed troubleshooting, search for error code: {ErrorCode}", ex.ErrorCode); - logger.LogInformation(""); - return; - } - catch (Exception ex) - { - logger.LogError(ex, "Failed to acquire MOS token: {Message}", ex.Message); - return; - } - - if (string.IsNullOrWhiteSpace(mosToken)) - { - logger.LogError("Unable to acquire MOS token. Aborting publish."); - return; - } - - using var http = HttpClientFactory.CreateAuthenticatedClient(mosToken, correlationId: correlationId); - - // Log token info for debugging (first/last chars only for security) - if (mosToken.Length >= 20) - { - var prefixLen = Math.Min(10, mosToken.Length / 2); - var suffixLen = Math.Min(10, mosToken.Length / 2); - logger.LogDebug("Using MOS token: {TokenStart}...{TokenEnd} (length: {Length})", - mosToken[..prefixLen], mosToken[^suffixLen..], mosToken.Length); - } - - // Step 2: POST packages (multipart form) - using tenant-specific URL - logger.LogInformation("Uploading package to Titles service..."); - var packagesUrl = $"{mosTitlesBaseUrl}/admin/v1/tenants/packages"; - logger.LogDebug("Upload URL: {Url}", packagesUrl); - logger.LogDebug("Package file: {ZipPath}", zipPath); - using var form = new MultipartFormDataContent(); - await using (var zipFs = File.OpenRead(zipPath)) - { - var fileContent = new StreamContent(zipFs); - fileContent.Headers.ContentType = new MediaTypeHeaderValue("application/zip"); - form.Add(fileContent, "package", Path.GetFileName(zipPath)); - - HttpResponseMessage uploadResp; - try - { - uploadResp = await http.PostAsync(packagesUrl, form); - } - catch (HttpRequestException ex) - { - logger.LogError("Network error during package upload: {Message}", ex.Message); - logger.LogInformation("The manifest package is available at: {ZipPath}", zipPath); - logger.LogInformation("You can manually upload it at: {Url}", packagesUrl); - logger.LogInformation("When network connectivity is restored, you can retry the publish command."); - return; - } - catch (TaskCanceledException ex) - { - logger.LogError("Upload request timed out: {Message}", ex.Message); - logger.LogInformation("The manifest package is available at: {ZipPath}", zipPath); - logger.LogInformation("You can manually upload it at: {Url}", packagesUrl); - logger.LogInformation("When network connectivity is restored, you can retry the publish command."); - return; - } - - var uploadBody = await uploadResp.Content.ReadAsStringAsync(); - logger.LogInformation("Titles upload HTTP {StatusCode}. Raw body length={Length} bytes", (int)uploadResp.StatusCode, uploadBody?.Length ?? 0); - if (!uploadResp.IsSuccessStatusCode) - { - logger.LogError("Package upload failed ({Status}). Body:\n{Body}", uploadResp.StatusCode, uploadBody); - - // Log response headers for additional diagnostic info - logger.LogDebug("Response headers:"); - foreach (var header in uploadResp.Headers) - { - logger.LogDebug(" {HeaderName}: {HeaderValue}", header.Key, string.Join(", ", header.Value)); - } - foreach (var header in uploadResp.Content.Headers) - { - logger.LogDebug(" {HeaderName}: {HeaderValue}", header.Key, string.Join(", ", header.Value)); - } - - // Provide helpful troubleshooting info for 401 - if (uploadResp.StatusCode == System.Net.HttpStatusCode.Unauthorized) - { - logger.LogError(""); - logger.LogError("TROUBLESHOOTING 401 UNAUTHORIZED:"); - logger.LogError("1. Verify MOS API permissions are configured correctly"); - logger.LogError(" - Required permission: Title.ReadWrite.All"); - logger.LogError(" - Admin consent must be granted"); - logger.LogError("2. Check that the token contains the correct scopes"); - logger.LogError(" - Run 'a365 publish -v' to see token scopes in debug logs"); - logger.LogError("3. Ensure you're signed in with the correct account"); - logger.LogError(" - Run 'az account show' to verify current account"); - logger.LogError("4. Try clearing the MOS token cache and re-authenticating:"); - logger.LogError(" - Delete: {CachePath}", Path.Combine(FileHelper.GetSecureCrossOsDirectory(), "mos-token-cache.json")); - logger.LogError(" - Run: a365 publish"); - logger.LogError(""); - } - - return; - } - - JsonDocument? uploadJson = null; - try - { - if (string.IsNullOrWhiteSpace(uploadBody)) - { - logger.LogError("Upload response body is null or empty. Cannot parse JSON."); - return; - } - uploadJson = JsonDocument.Parse(uploadBody); - } - catch (Exception jex) - { - logger.LogError(jex, "Failed to parse upload response JSON. Body was:\n{Body}", uploadBody); - return; - } - // Extract operationId (required) - if (!uploadJson.RootElement.TryGetProperty("operationId", out var opIdEl)) - { - var propertyNames = string.Join( - ", ", - uploadJson.RootElement.EnumerateObject().Select(p => p.Name)); - logger.LogError("operationId missing in upload response. Present properties: [{Props}] Raw body:\n{Body}", propertyNames, uploadBody); - return; - } - var operationId = opIdEl.GetString(); - if (string.IsNullOrWhiteSpace(operationId)) - { - logger.LogError("operationId property empty/null. Raw body:\n{Body}", uploadBody); - return; - } - // Extract titleId only from titlePreview block - string? titleId = null; - if (uploadJson.RootElement.TryGetProperty("titlePreview", out var previewEl) && - previewEl.ValueKind == JsonValueKind.Object && - previewEl.TryGetProperty("titleId", out var previewTitleIdEl)) - { - titleId = previewTitleIdEl.GetString(); - } - if (string.IsNullOrWhiteSpace(titleId)) - { - logger.LogError("titleId not found under titlePreview.titleId. Raw body:\n{Body}", uploadBody); - return; - } - - logger.LogInformation("Upload succeeded. operationId={Op} titleId={Title}", operationId, titleId); - - logger.LogDebug("Proceeding to title creation step..."); - - // POST titles with operationId - using tenant-specific URL - var titlesUrl = $"{mosTitlesBaseUrl}/admin/v1/tenants/packages/titles"; - logger.LogDebug("Title creation URL: {Url}", titlesUrl); - var titlePayload = JsonSerializer.Serialize(new { operationId }); - - HttpResponseMessage titlesResp; - try - { - using (var content = new StringContent(titlePayload, System.Text.Encoding.UTF8, "application/json")) - { - titlesResp = await http.PostAsync(titlesUrl, content); - } - } - catch (HttpRequestException ex) - { - logger.LogError("Network error during title creation: {Message}", ex.Message); - logger.LogInformation("Package was uploaded successfully (operationId={Op}), but title creation failed.", operationId); - return; - } - catch (TaskCanceledException ex) - { - logger.LogError("Title creation request timed out: {Message}", ex.Message); - logger.LogInformation("Package was uploaded successfully (operationId={Op}), but title creation failed.", operationId); - return; - } - - var titlesBody = await titlesResp.Content.ReadAsStringAsync(); - if (!titlesResp.IsSuccessStatusCode) - { - logger.LogError("Titles creation failed ({Status}). Payload sent={Payload}. Body:\n{Body}", titlesResp.StatusCode, titlePayload, titlesBody); - return; - } - logger.LogInformation("Title creation initiated. Response body length={Length} bytes", titlesBody?.Length ?? 0); - - // Wait 10 seconds before allowing all users to ensure title is fully created - logger.LogInformation("Configuring title access for all users with retry and exponential backoff..."); - var allowUrl = $"{mosTitlesBaseUrl}/admin/v1/tenants/titles/{titleId}/allowed"; - var allowedPayload = JsonSerializer.Serialize(new - { - EntityCollection = new - { - ForAllUsers = true, - Entities = Array.Empty() - } - }); - - // Use custom retry helper - var retryHelper = new RetryHelper(logger); - - var allowResult = await retryHelper.ExecuteWithRetryAsync( - async ct => - { - using var content = new StringContent(allowedPayload, System.Text.Encoding.UTF8, "application/json"); - var resp = await http.PostAsync(allowUrl, content, ct); - var body = await resp.Content.ReadAsStringAsync(ct); - return (resp, body); - }, - result => - { - var (resp, body) = result; - - if (resp.IsSuccessStatusCode) - { - return false; - } - - if ((int)resp.StatusCode == 404 && body.Contains("Title Not Found", StringComparison.OrdinalIgnoreCase)) - { - logger.LogWarning("Title not found yet (HTTP 404). Will retry..."); - return true; - } - - return false; - }, - maxRetries: 5, - baseDelaySeconds: 10, - CancellationToken.None); - - var (allowResp, allowBody) = allowResult; - if (!allowResp.IsSuccessStatusCode) - { - logger.LogError("Allow users failed ({Status}). URL={Url} Payload={Payload} Body:\n{Body}", allowResp.StatusCode, allowUrl, allowedPayload, allowBody); - return; - } - logger.LogInformation("Title access configured for all users. Allow response length={Length} bytes", allowBody?.Length ?? 0); - logger.LogDebug("Allow users response body:\n{Body}", allowBody); - } - - // ================= Graph API Operations ================= - if (skipGraph) - { - logger.LogInformation("--skip-graph specified; skipping federated identity credential and role assignment."); - isNormalExit = true; - return; - } - - if (string.IsNullOrWhiteSpace(tenantId)) - { - logger.LogWarning("tenantId unavailable; skipping Graph operations."); - // Treat as normal exit (exit code 0) because MOS publish completed successfully - // and Graph operations are optional. Users who need Graph operations should ensure - // tenantId is configured or use --skip-graph explicitly. - isNormalExit = true; - return; - } - - logger.LogDebug("Configuring Graph API permissions (federated identity and role assignments)..."); - logger.LogInformation("Executing Graph API operations..."); - logger.LogInformation("TenantId: {TenantId}, BlueprintId: {BlueprintId}", tenantId, blueprintId); - - var graphSuccess = await agentPublishService.ExecutePublishGraphStepsAsync( - tenantId, - blueprintId, - blueprintId, // Using blueprintId as manifestId - CancellationToken.None); - - if (!graphSuccess) - { - logger.LogError("Graph API operations failed"); - return; - } + // Print manual upload instructions + logger.LogInformation(""); + logger.LogInformation("=== NEXT STEP: UPLOAD YOUR AGENT ==="); + logger.LogInformation(""); + logger.LogInformation("Your manifest package is ready at:"); + Console.WriteLine($" {zipPath}"); + logger.LogInformation(""); + logger.LogInformation("To publish your agent to Microsoft 365:"); + logger.LogInformation(" 1. Go to the Microsoft 365 Admin Center (https://admin.microsoft.com)"); + logger.LogInformation(" 2. Navigate to Agents > All agents"); + logger.LogInformation(" 3. Click 'Upload custom agent' and upload the manifest.zip file"); + logger.LogInformation(""); + logger.LogInformation("For detailed upload instructions, see:"); + logger.LogInformation(" https://learn.microsoft.com/en-us/copilot/microsoft-365/agent-essentials/agent-lifecycle/agent-upload-agents"); + logger.LogInformation(""); - logger.LogInformation("Publish completed successfully!"); isNormalExit = true; } catch (Exception ex) @@ -689,10 +282,6 @@ public static Command CreateCommand( } finally { - // Set exit code 1 for all error paths (different from ConfigCommand's per-site approach, - // but more robust as it catches all error returns and exceptions automatically). - // This ensures any error path that doesn't explicitly set isNormalExit=true will - // return exit code 1, preventing the bug where ~27 error paths returned 0. if (!isNormalExit) { context.ExitCode = 1; @@ -767,4 +356,3 @@ private static async Task UpdateAgenticUserManifestTemplateFileAsync(ILo } } - diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ErrorMessages.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ErrorMessages.cs index 2079a944..a20b692b 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ErrorMessages.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ErrorMessages.cs @@ -182,48 +182,5 @@ public static List GetMosServicePrincipalMitigation(string appId) }; } - /// - /// Gets mitigation steps for first-party client app service principal creation. - /// - public static List GetFirstPartyClientAppServicePrincipalMitigation() - { - return new List - { - "Insufficient privileges to create service principal for Microsoft first-party client app.", - "This app is required for MOS token acquisition.", - "Required role: Application Administrator, Cloud Application Administrator, or Global Administrator.", - $"Ask your tenant administrator to run: az ad sp create --id {MosConstants.TpsAppServicesClientAppId}" - }; - } - - /// - /// Gets mitigation steps for all MOS resource app service principals. - /// - public static List GetMosResourceAppsServicePrincipalMitigation() - { - return new List - { - "Insufficient privileges to create service principals for MOS resource applications.", - "Required role: Application Administrator, Cloud Application Administrator, or Global Administrator.", - "Ask your tenant administrator to run:", - " az ad sp create --id 6ec511af-06dc-4fe2-b493-63a37bc397b1", - " az ad sp create --id 8578e004-a5c6-46e7-913e-12f58912df43", - " az ad sp create --id e8be65d6-d430-4289-a665-51bf2a194bda" - }; - } - - /// - /// Gets mitigation steps for MOS admin consent issues. - /// - public static List GetMosAdminConsentMitigation(string clientAppId) - { - return new List - { - "Admin consent required for MOS API permissions.", - $"Grant consent at: https://portal.azure.com/#view/Microsoft_AAD_RegisteredApps/ApplicationMenuBlade/~/CallAnAPI/appId/{clientAppId}", - "Click 'Grant admin consent for [Your Organization]' and wait 1-2 minutes for propagation." - }; - } - #endregion } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Constants/MosConstants.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Constants/MosConstants.cs index 88746503..d57595ee 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Constants/MosConstants.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Constants/MosConstants.cs @@ -4,210 +4,23 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Constants; /// -/// Constants for Microsoft Office Store (MOS) API authentication and permissions +/// Constants for Microsoft Power Platform API authentication and permissions /// public static class MosConstants { /// - /// Redirect URI for MOS token acquisition (aligns with custom client app configuration) - /// - public const string RedirectUri = "http://localhost:8400/"; - - /// - /// Authority URL for commercial cloud MOS authentication - /// - public const string CommercialAuthority = "https://login.microsoftonline.com/common"; - - /// - /// Authority URL for US Government cloud MOS authentication (GCCH/DOD) - /// - public const string GovernmentAuthority = "https://login.microsoftonline.us/common"; - - /// - /// TPS AppServices 3p App (Client) - Microsoft first-party client app ID - /// Used for MOS token acquisition (NOT the custom client app) - /// - public const string TpsAppServicesClientAppId = "caef0b02-8d39-46ab-b28c-f517033d8a21"; - - /// - /// TPS AppServices 3p App (Server) resource app ID - /// Required for test environment token acquisition - /// - public const string TpsAppServicesResourceAppId = "6ec511af-06dc-4fe2-b493-63a37bc397b1"; - - /// - /// Power Platform API resource app ID for MOS token + /// Power Platform API resource app ID /// public const string PowerPlatformApiResourceAppId = "8578e004-a5c6-46e7-913e-12f58912df43"; /// - /// MOS Titles API resource app ID - /// Required for accessing MOS Titles service (titles.prod.mos.microsoft.com) - /// - public const string MosTitlesApiResourceAppId = "e8be65d6-d430-4289-a665-51bf2a194bda"; - - /// - /// All MOS resource app IDs that need service principals created in the tenant - /// - public static readonly string[] AllResourceAppIds = new[] - { - TpsAppServicesResourceAppId, - PowerPlatformApiResourceAppId, - MosTitlesApiResourceAppId - }; - - /// - /// Delegated permission scope IDs for MOS resource applications. - /// These IDs are stable Microsoft-defined GUIDs that identify specific OAuth2 permissions. - /// To verify these IDs are current, use Graph API or Azure Portal: - /// - Graph API: GET https://graph.microsoft.com/v1.0/servicePrincipals(appId='{resourceAppId}')?$select=oauth2PermissionScopes - /// - Azure Portal: Enterprise Applications > Search by App ID > Permissions - /// - public static class PermissionIds - { - /// - /// TPS AppServices - AuthConfig.Read permission scope ID - /// - public const string TpsAppServicesAuthConfigRead = "6f17ed22-2455-4cfc-a02d-9ccdde5f7f8c"; - - /// - /// Power Platform API - EnvironmentManagement.Environments.Read permission scope ID - /// - public const string PowerPlatformEnvironmentsRead = "177690ed-85f1-41d9-8dbf-2716e60ff46a"; - - /// - /// MOS Titles API - Title.ReadWrite.All permission scope ID - /// - public const string MosTitlesTitleReadWriteAll = "ecb8a615-f488-4c95-9efe-cb0142fc07dd"; - - /// - /// Power Platform API - CopilotStudio.Copilots.Invoke permission scope ID - /// - public const string PowerPlatformCopilotStudioInvoke = "204440d3-c1d0-4826-b570-99eb6f5e2aeb"; - } - - /// - /// Delegated permission scope names for MOS resource applications. + /// Delegated permission scope names for resource applications. /// public static class PermissionNames { - /// - /// TPS AppServices - AuthConfig.Read permission scope name - /// - public const string TpsAppServicesAuthConfigRead = "AuthConfig.Read"; - - /// - /// Power Platform API - EnvironmentManagement.Environments.Read permission scope name - /// - public const string PowerPlatformEnvironmentsRead = "EnvironmentManagement.Environments.Read"; - - /// - /// MOS Titles API - Title.ReadWrite.All permission scope name - /// - public const string MosTitlesTitleReadWriteAll = "Title.ReadWrite.All"; - /// /// Power Platform API - CopilotStudio.Copilots.Invoke permission scope name /// public const string PowerPlatformCopilotStudioInvoke = "CopilotStudio.Copilots.Invoke"; } - - /// - /// Complete permission configuration for each MOS resource app. - /// Each entry contains: resource app ID, scope name for OAuth2 grants, and scope ID for requiredResourceAccess. - /// This centralized mapping ensures consistency between requiredResourceAccess configuration and OAuth2 permission grants. - /// - public static class ResourcePermissions - { - /// - /// Permission configuration for TPS AppServices resource app. - /// Required for test environment MOS operations. - /// - public static readonly (string ResourceAppId, string ScopeName, string ScopeId) TpsAppServices = - (TpsAppServicesResourceAppId, PermissionNames.TpsAppServicesAuthConfigRead, PermissionIds.TpsAppServicesAuthConfigRead); - - /// - /// Permission configuration for Power Platform API resource app. - /// Required for environment management operations. - /// - public static readonly (string ResourceAppId, string ScopeName, string ScopeId) PowerPlatformApi = - (PowerPlatformApiResourceAppId, PermissionNames.PowerPlatformEnvironmentsRead, PermissionIds.PowerPlatformEnvironmentsRead); - - /// - /// Permission configuration for MOS Titles API resource app. - /// Uses the primary Title.ReadWrite.All scope that corresponds to the specified ScopeId. - /// - public static readonly (string ResourceAppId, string ScopeName, string ScopeId) MosTitlesApi = - (MosTitlesApiResourceAppId, PermissionNames.MosTitlesTitleReadWriteAll, PermissionIds.MosTitlesTitleReadWriteAll); - - /// - /// Permission configuration for Power Platform API - CopilotStudio. - /// Required for agent blueprints to invoke Copilot Studio copilots. - /// - public static readonly (string ResourceAppId, string ScopeName, string ScopeId) CopilotStudioApi = - (PowerPlatformApiResourceAppId, PermissionNames.PowerPlatformCopilotStudioInvoke, PermissionIds.PowerPlatformCopilotStudioInvoke); - - /// - /// Gets all resource permission configurations. - /// Use this to iterate over all MOS resource apps during setup. - /// - public static IEnumerable<(string ResourceAppId, string ScopeName, string ScopeId)> GetAll() - { - yield return TpsAppServices; - yield return PowerPlatformApi; - yield return MosTitlesApi; - } - } - - /// - /// MOS environment configuration mapping - /// Maps environment names to their scope URLs - /// - public static class Environments - { - public const string Prod = "prod"; - public const string Sdf = "sdf"; - public const string Test = "test"; - public const string Gccm = "gccm"; - public const string Gcch = "gcch"; - public const string Dod = "dod"; - - /// - /// Scope for production MOS environment - /// - public const string ProdScope = "https://titles.prod.mos.microsoft.com/.default"; - - /// - /// Scope for SDF MOS environment - /// - public const string SdfScope = "https://titles.sdf.mos.microsoft.com/.default"; - - /// - /// Scope for test MOS environment - /// - public const string TestScope = "https://testappservices.mos.microsoft.com/.default"; - - /// - /// Scope for GCCM MOS environment - /// - public const string GccmScope = "https://titles.gccm.mos.microsoft.com/.default"; - - /// - /// Scope for GCCH MOS environment - /// - public const string GcchScope = "https://titles.gcch.mos.svc.usgovcloud.microsoft/.default"; - - /// - /// Scope for DOD MOS environment - /// - public const string DodScope = "https://titles.dod.mos.svc.usgovcloud.microsoft/.default"; - } - - /// - /// Generates Azure Portal URL for API permissions page of a specific client app - /// - public static string GetApiPermissionsPortalUrl(string clientAppId) - { - return $"https://portal.azure.com/#view/Microsoft_AAD_RegisteredApps/ApplicationMenuBlade/~/CallAnAPI/appId/{clientAppId}"; - } } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/PublishHelpers.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/PublishHelpers.cs deleted file mode 100644 index 9853479a..00000000 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/PublishHelpers.cs +++ /dev/null @@ -1,590 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -using Microsoft.Extensions.Logging; -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; - -namespace Microsoft.Agents.A365.DevTools.Cli.Helpers; - -/// -/// Helper methods for publish command operations -/// -public static class PublishHelpers -{ - /// - /// Checks if all MOS prerequisites are already configured (idempotency check). - /// Returns true if service principals, permissions, and admin consent are all in place. - /// - private static async Task CheckMosPrerequisitesAsync( - GraphApiService graph, - Agent365Config config, - System.Text.Json.JsonElement app, - ILogger logger, - CancellationToken ct) - { - // Check 1: Verify all required service principals exist - // Cache SP IDs so Check 3 can reuse them without redundant Graph API lookups. - var firstPartyClientSpId = await graph.LookupServicePrincipalByAppIdAsync(config.TenantId, - MosConstants.TpsAppServicesClientAppId, ct); - if (string.IsNullOrWhiteSpace(firstPartyClientSpId)) - { - logger.LogDebug("Service principal for {ConstantName} ({AppId}) not found - configuration needed", - nameof(MosConstants.TpsAppServicesClientAppId), MosConstants.TpsAppServicesClientAppId); - return false; - } - logger.LogDebug("Verified service principal for {ConstantName} ({AppId})", - nameof(MosConstants.TpsAppServicesClientAppId), MosConstants.TpsAppServicesClientAppId); - - var resourceSpIds = new Dictionary(StringComparer.OrdinalIgnoreCase); - foreach (var resourceAppId in MosConstants.AllResourceAppIds) - { - var spId = await graph.LookupServicePrincipalByAppIdAsync(config.TenantId, resourceAppId, ct); - if (string.IsNullOrWhiteSpace(spId)) - { - logger.LogDebug("Service principal for {ResourceAppId} not found - configuration needed", resourceAppId); - return false; - } - resourceSpIds[resourceAppId] = spId; - logger.LogDebug("Verified service principal for resource app ({ResourceAppId})", resourceAppId); - } - - // Check 2: Verify all MOS permissions are in requiredResourceAccess with correct scopes - var mosResourcePermissions = MosConstants.ResourcePermissions.GetAll() - .ToDictionary(p => p.ResourceAppId, p => (p.ScopeName, p.ScopeId)); - - if (!app.TryGetProperty("requiredResourceAccess", out var currentResourceAccess)) - { - logger.LogDebug("No requiredResourceAccess found - configuration needed"); - return false; - } - - var existingResources = currentResourceAccess.EnumerateArray() - .Where(r => r.TryGetProperty("resourceAppId", out var _)) - .ToList(); - - foreach (var (resourceAppId, (scopeName, scopeId)) in mosResourcePermissions) - { - var existingResource = existingResources - .FirstOrDefault(r => r.GetProperty("resourceAppId").GetString() == resourceAppId); - - if (existingResource.ValueKind == System.Text.Json.JsonValueKind.Undefined) - { - logger.LogDebug("MOS resource {ResourceAppId} not in requiredResourceAccess - configuration needed", resourceAppId); - return false; - } - - // Verify the correct scope is present - if (!existingResource.TryGetProperty("resourceAccess", out var resourceAccessArray)) - { - logger.LogDebug("MOS resource {ResourceAppId} missing resourceAccess - configuration needed", resourceAppId); - return false; - } - - var hasCorrectScope = resourceAccessArray.EnumerateArray() - .Where(permission => permission.TryGetProperty("id", out var _)) - .Any(permission => permission.GetProperty("id").GetString() == scopeId); - - if (!hasCorrectScope) - { - logger.LogDebug("MOS resource {ResourceAppId} missing correct scope {ScopeName} - configuration needed", - resourceAppId, scopeName); - return false; - } - - logger.LogDebug("Verified permission {ScopeName} ({ScopeId}) for resource app ({ResourceAppId})", - scopeName, scopeId, resourceAppId); - } - - // Check 3: Verify admin consent is granted for all MOS resources - // Reuse SP IDs cached from Check 1 to avoid redundant Graph API lookups. - var mosResourceScopes = MosConstants.ResourcePermissions.GetAll() - .ToDictionary(p => p.ResourceAppId, p => p.ScopeName); - - foreach (var (resourceAppId, scopeName) in mosResourceScopes) - { - if (!resourceSpIds.TryGetValue(resourceAppId, out var resourceSpId)) - { - logger.LogWarning("Service principal for {ResourceAppId} not found in cache (unexpected - should have been cached by Check 1)", resourceAppId); - return false; - } - - // Check if OAuth2 permission grant exists - var grantDoc = await graph.GraphGetAsync(config.TenantId, - $"/v1.0/oauth2PermissionGrants?$filter=clientId eq '{firstPartyClientSpId}' and resourceId eq '{resourceSpId}'", - ct); - - if (grantDoc == null || !grantDoc.RootElement.TryGetProperty("value", out var grants) || grants.GetArrayLength() == 0) - { - logger.LogDebug("Admin consent not granted for {ResourceAppId} - configuration needed", resourceAppId); - return false; - } - - // Verify the grant has the correct scope - var grant = grants[0]; - if (!grant.TryGetProperty("scope", out var grantedScopes)) - { - logger.LogDebug("Admin consent for {ResourceAppId} missing scope property - configuration needed", resourceAppId); - return false; - } - - var scopesString = grantedScopes.GetString(); - if (string.IsNullOrWhiteSpace(scopesString) || !scopesString.Contains(scopeName)) - { - logger.LogDebug("Admin consent for {ResourceAppId} missing scope {ScopeName} - configuration needed", - resourceAppId, scopeName); - return false; - } - - logger.LogDebug("Verified admin consent for resource app ({ResourceAppId}) with scope {ScopeName}", - resourceAppId, scopeName); - } - - // All checks passed - logger.LogDebug("All MOS prerequisites verified: {Count} service principals, {Count} permissions, {Count} admin consents", - MosConstants.AllResourceAppIds.Length + 1, MosConstants.AllResourceAppIds.Length, MosConstants.AllResourceAppIds.Length); - return true; - } - - /// - /// Ensures all required service principals exist for MOS access. - /// Idempotent - only creates service principals that don't already exist. - /// - private static async Task EnsureMosServicePrincipalsAsync( - GraphApiService graph, - Agent365Config config, - ILogger logger, - CancellationToken ct) - { - // Check 1: First-party client app service principal - var firstPartySpId = await graph.LookupServicePrincipalByAppIdAsync(config.TenantId, - MosConstants.TpsAppServicesClientAppId, ct); - - if (string.IsNullOrWhiteSpace(firstPartySpId)) - { - logger.LogInformation("Creating service principal for Microsoft first-party client app..."); - - try - { - firstPartySpId = await graph.EnsureServicePrincipalForAppIdAsync(config.TenantId, - MosConstants.TpsAppServicesClientAppId, ct); - - if (string.IsNullOrWhiteSpace(firstPartySpId)) - { - throw new SetupValidationException( - $"Failed to create service principal for Microsoft first-party client app {MosConstants.TpsAppServicesClientAppId}.", - mitigationSteps: ErrorMessages.GetFirstPartyClientAppServicePrincipalMitigation()); - } - - logger.LogDebug("Created first-party client app service principal: {SpObjectId}", firstPartySpId); - } - catch (Exception ex) when (ex is not SetupValidationException) - { - logger.LogError(ex, "Failed to create service principal for first-party client app"); - - if (ex.Message.Contains("403") || ex.Message.Contains("Insufficient privileges") || - ex.Message.Contains("Authorization_RequestDenied")) - { - throw new SetupValidationException( - "Insufficient privileges to create service principal for Microsoft first-party client app.", - mitigationSteps: ErrorMessages.GetFirstPartyClientAppServicePrincipalMitigation()); - } - - throw new SetupValidationException($"Failed to create service principal for first-party client app: {ex.Message}"); - } - } - else - { - logger.LogDebug("First-party client app service principal already exists: {SpObjectId}", firstPartySpId); - } - - // Check 2: MOS resource app service principals - var missingResourceApps = new List(); - foreach (var resourceAppId in MosConstants.AllResourceAppIds) - { - var spId = await graph.LookupServicePrincipalByAppIdAsync(config.TenantId, resourceAppId, ct); - if (string.IsNullOrWhiteSpace(spId)) - { - missingResourceApps.Add(resourceAppId); - } - } - - if (missingResourceApps.Count > 0) - { - logger.LogInformation("Creating service principals for {Count} MOS resource applications...", missingResourceApps.Count); - - foreach (var resourceAppId in missingResourceApps) - { - try - { - var spId = await graph.EnsureServicePrincipalForAppIdAsync(config.TenantId, resourceAppId, ct); - - if (string.IsNullOrWhiteSpace(spId)) - { - throw new SetupValidationException( - $"Failed to create service principal for MOS resource app {resourceAppId}.", - mitigationSteps: ErrorMessages.GetMosServicePrincipalMitigation(resourceAppId)); - } - - logger.LogDebug("Created service principal for {ResourceAppId}: {SpObjectId}", resourceAppId, spId); - } - catch (Exception ex) when (ex is not SetupValidationException) - { - logger.LogError(ex, "Failed to create service principal for MOS resource app {ResourceAppId}", resourceAppId); - - if (ex.Message.Contains("403") || ex.Message.Contains("Insufficient privileges") || - ex.Message.Contains("Authorization_RequestDenied")) - { - throw new SetupValidationException( - $"Insufficient privileges to create service principal for MOS resource app {resourceAppId}.", - mitigationSteps: ErrorMessages.GetMosServicePrincipalMitigation(resourceAppId)); - } - - throw new SetupValidationException($"Failed to create service principal for MOS resource app {resourceAppId}: {ex.Message}"); - } - } - } - else - { - logger.LogDebug("All MOS resource app service principals already exist"); - } - } - - /// - /// Ensures MOS permissions are configured in custom client app's requiredResourceAccess. - /// Idempotent - only updates if permissions are missing or incorrect. - /// - private static async Task EnsureMosPermissionsConfiguredAsync( - GraphApiService graph, - Agent365Config config, - System.Text.Json.JsonElement app, - ILogger logger, - CancellationToken ct) - { - if (!app.TryGetProperty("id", out var appObjectIdElement)) - { - throw new SetupValidationException($"Application {config.ClientAppId} missing id property"); - } - var appObjectId = appObjectIdElement.GetString()!; - - // Get existing requiredResourceAccess - var resourceAccessList = new List(); - if (app.TryGetProperty("requiredResourceAccess", out var currentResourceAccess)) - { - resourceAccessList = currentResourceAccess.EnumerateArray().ToList(); - } - - var mosResourcePermissions = MosConstants.ResourcePermissions.GetAll() - .ToDictionary(p => p.ResourceAppId, p => (p.ScopeName, p.ScopeId)); - - // Check what needs to be added or fixed - var needsUpdate = false; - var updatedResourceAccess = new List(); - var processedMosResources = new HashSet(); - - // Process existing resources - foreach (var existingResource in resourceAccessList) - { - if (!existingResource.TryGetProperty("resourceAppId", out var resAppIdProp)) - { - continue; - } - - var existingResourceAppId = resAppIdProp.GetString(); - if (string.IsNullOrEmpty(existingResourceAppId)) - { - continue; - } - - if (MosConstants.AllResourceAppIds.Contains(existingResourceAppId)) - { - var (expectedScopeName, expectedScopeId) = mosResourcePermissions[existingResourceAppId]; - var hasCorrectPermission = false; - - if (existingResource.TryGetProperty("resourceAccess", out var resourceAccessArray)) - { - hasCorrectPermission = resourceAccessArray.EnumerateArray() - .Where(permission => permission.TryGetProperty("id", out var _)) - .Any(permission => permission.GetProperty("id").GetString() == expectedScopeId); - } - - if (hasCorrectPermission) - { - logger.LogDebug("MOS resource app {ResourceAppId} already has correct permission", existingResourceAppId); - var resourceObj = System.Text.Json.JsonSerializer.Deserialize(existingResource.GetRawText()); - if (resourceObj != null) - { - updatedResourceAccess.Add(resourceObj); - } - } - else - { - logger.LogDebug("Fixing permission for MOS resource app {ResourceAppId}", existingResourceAppId); - needsUpdate = true; - updatedResourceAccess.Add(new - { - resourceAppId = existingResourceAppId, - resourceAccess = new[] - { - new { id = expectedScopeId, type = "Scope" } - } - }); - } - - processedMosResources.Add(existingResourceAppId); - } - else - { - // Non-MOS resource - preserve as-is - var resourceObj = System.Text.Json.JsonSerializer.Deserialize(existingResource.GetRawText()); - if (resourceObj != null) - { - updatedResourceAccess.Add(resourceObj); - } - } - } - - // Add missing MOS resources - var missingResources = MosConstants.AllResourceAppIds - .Where(id => !processedMosResources.Contains(id)) - .ToList(); - - if (missingResources.Count > 0) - { - logger.LogInformation("Adding {Count} missing MOS permissions to custom client app", missingResources.Count); - needsUpdate = true; - - foreach (var resourceAppId in missingResources) - { - var (scopeName, scopeId) = mosResourcePermissions[resourceAppId]; - logger.LogDebug("Adding MOS resource app {ResourceAppId} with scope {ScopeName}", resourceAppId, scopeName); - - updatedResourceAccess.Add(new - { - resourceAppId = resourceAppId, - resourceAccess = new[] - { - new { id = scopeId, type = "Scope" } - } - }); - } - } - - // Only update if something changed - if (!needsUpdate) - { - logger.LogDebug("MOS permissions already configured correctly"); - return; - } - - try - { - var patchPayload = new { requiredResourceAccess = updatedResourceAccess }; - logger.LogDebug("Updating application {AppObjectId} with {Count} resource access entries", - appObjectId, updatedResourceAccess.Count); - - var updated = await graph.GraphPatchAsync(config.TenantId, $"/v1.0/applications/{appObjectId}", patchPayload, ct); - if (!updated) - { - throw new SetupValidationException("Failed to update application with MOS API permissions."); - } - - logger.LogInformation("MOS API permissions configured successfully"); - } - catch (Exception ex) when (ex is not SetupValidationException) - { - logger.LogError(ex, "Error configuring MOS API permissions"); - throw new SetupValidationException($"Failed to configure MOS API permissions: {ex.Message}"); - } - } - - /// - /// Ensures MOS (Microsoft Online Services) prerequisites are configured for the custom client app. - /// This includes creating service principals for MOS resource apps and verifying admin consent. - /// - /// Graph API service for making Microsoft Graph calls - /// Agent365 configuration containing tenant and client app information - /// Logger for diagnostic output - /// Cancellation token - /// True if prerequisites are configured successfully - /// Thrown when prerequisites cannot be configured - public static async Task EnsureMosPrerequisitesAsync( - GraphApiService graph, - AgentBlueprintService blueprintService, - Agent365Config config, - ILogger logger, - CancellationToken ct = default) - { - if (string.IsNullOrWhiteSpace(config.ClientAppId)) - { - logger.LogError("Custom client app ID not found in configuration. Run 'a365 config init' first."); - throw new SetupValidationException("Custom client app ID is required for MOS token acquisition."); - } - - // Load custom client app - logger.LogDebug("Checking MOS prerequisites for custom client app {ClientAppId}", config.ClientAppId); - var appDoc = await graph.GraphGetAsync(config.TenantId, - $"/v1.0/applications?$filter=appId eq '{config.ClientAppId}'&$select=id,requiredResourceAccess", ct); - - if (appDoc == null || !appDoc.RootElement.TryGetProperty("value", out var appsArray) || appsArray.GetArrayLength() == 0) - { - logger.LogError("Custom client app {ClientAppId} not found in tenant", config.ClientAppId); - throw new SetupValidationException($"Custom client app {config.ClientAppId} not found. Verify the app exists and you have access."); - } - - var app = appsArray[0]; - - // Check if all MOS prerequisites are already configured (idempotency check) - var prerequisitesMet = await CheckMosPrerequisitesAsync(graph, config, app, logger, ct); - if (prerequisitesMet) - { - logger.LogDebug("MOS prerequisites already configured"); - return true; - } - - logger.LogDebug("Configuring MOS API prerequisites..."); - - // Step 1: Ensure service principals exist (idempotent - only creates if missing) - await EnsureMosServicePrincipalsAsync(graph, config, logger, ct); - - // Step 2: Ensure MOS permissions are configured in requiredResourceAccess (idempotent - only updates if needed) - await EnsureMosPermissionsConfiguredAsync(graph, config, app, logger, ct); - - // Step 3: Ensure admin consent is granted for MOS permissions (idempotent - only grants if missing) - await EnsureMosAdminConsentAsync(graph, blueprintService, config, logger, ct); - - return true; - } - - /// - /// Ensures admin consent is granted for MOS permissions. - /// Idempotent - only grants consent for resources that don't already have it. - /// Uses the Microsoft first-party client app for MOS access (required by MOS APIs). - /// - private static async Task EnsureMosAdminConsentAsync( - GraphApiService graph, - AgentBlueprintService blueprintService, - Agent365Config config, - ILogger logger, - CancellationToken ct) - { - // Look up the first-party client app's service principal - var clientSpObjectId = await graph.LookupServicePrincipalByAppIdAsync(config.TenantId, - MosConstants.TpsAppServicesClientAppId, ct); - - if (string.IsNullOrWhiteSpace(clientSpObjectId)) - { - throw new SetupValidationException( - $"Service principal not found for Microsoft first-party client app {MosConstants.TpsAppServicesClientAppId}"); - } - - logger.LogDebug("First-party client service principal ID: {ClientSpObjectId}", clientSpObjectId); - - var mosResourceScopes = MosConstants.ResourcePermissions.GetAll() - .ToDictionary(p => p.ResourceAppId, p => p.ScopeName); - - var resourcesToConsent = new List<(string ResourceAppId, string ScopeName, string ResourceSpId)>(); - - // Check which resources need consent - foreach (var (resourceAppId, scopeName) in mosResourceScopes) - { - var resourceSpObjectId = await graph.LookupServicePrincipalByAppIdAsync(config.TenantId, resourceAppId, ct); - if (string.IsNullOrWhiteSpace(resourceSpObjectId)) - { - logger.LogWarning("Service principal not found for MOS resource app {ResourceAppId} - skipping consent", resourceAppId); - continue; - } - - // Check if consent already exists - var grantDoc = await graph.GraphGetAsync(config.TenantId, - $"/v1.0/oauth2PermissionGrants?$filter=clientId eq '{clientSpObjectId}' and resourceId eq '{resourceSpObjectId}'", - ct); - - var hasConsent = false; - if (grantDoc != null && grantDoc.RootElement.TryGetProperty("value", out var grants) && grants.GetArrayLength() > 0) - { - var grant = grants[0]; - if (grant.TryGetProperty("scope", out var grantedScopes)) - { - var scopesString = grantedScopes.GetString(); - hasConsent = !string.IsNullOrWhiteSpace(scopesString) && scopesString.Contains(scopeName); - } - } - - if (hasConsent) - { - logger.LogDebug("Admin consent already granted for {ResourceAppId}", resourceAppId); - } - else - { - resourcesToConsent.Add((resourceAppId, scopeName, resourceSpObjectId)); - } - } - - // Grant consent for resources that need it - if (resourcesToConsent.Count == 0) - { - logger.LogDebug("Admin consent already configured for all MOS resources"); - return; - } - - logger.LogInformation("Granting admin consent for {Count} MOS resources", resourcesToConsent.Count); - - var failedGrants = new List(); - - foreach (var (resourceAppId, scopeName, resourceSpObjectId) in resourcesToConsent) - { - logger.LogDebug("Granting admin consent for {ResourceAppId} with scope {ScopeName}", resourceAppId, scopeName); - - var success = await blueprintService.ReplaceOauth2PermissionGrantAsync( - config.TenantId, - clientSpObjectId, - resourceSpObjectId, - new[] { scopeName }, - ct); - - if (!success) - { - logger.LogError("Failed to grant admin consent for {ResourceAppId}", resourceAppId); - failedGrants.Add(resourceAppId); - } - } - - if (failedGrants.Count > 0) - { - var failedList = string.Join(", ", failedGrants); - logger.LogError("Failed to grant admin consent for {Count} MOS resource(s): {FailedResources}", - failedGrants.Count, failedList); - throw new SetupValidationException( - $"Failed to grant admin consent for {failedGrants.Count} MOS resource(s): {failedList}. " + - "MOS token acquisition will fail without proper consent.", - mitigationSteps: ErrorMessages.GetMosAdminConsentMitigation(config.ClientAppId)); - } - - logger.LogInformation("Admin consent granted successfully for all {Count} MOS resources", resourcesToConsent.Count); - - // Clear cached MOS tokens to force re-acquisition with new scopes - logger.LogDebug("Clearing cached MOS tokens to force re-acquisition with updated permissions"); - var cacheDir = FileHelper.GetSecureCrossOsDirectory(); - var cacheFilePath = Path.Combine(cacheDir, "mos-token-cache.json"); - if (File.Exists(cacheFilePath)) - { - try - { - File.Delete(cacheFilePath); - logger.LogDebug("Deleted MOS token cache file: {CacheFile}", cacheFilePath); - } - catch (Exception ex) - { - logger.LogWarning("Could not delete MOS token cache file {CacheFile}: {Message}", - cacheFilePath, ex.Message); - } - } - else - { - logger.LogDebug("No MOS token cache file found at {CacheFile}", cacheFilePath); - } - } -} diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs index fa64b85b..fc78987a 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs @@ -98,7 +98,6 @@ static async Task Main(string[] args) var deploymentService = serviceProvider.GetRequiredService(); var botConfigurator = serviceProvider.GetRequiredService(); var graphApiService = serviceProvider.GetRequiredService(); - var agentPublishService = serviceProvider.GetRequiredService(); var agentBlueprintService = serviceProvider.GetRequiredService(); var blueprintLookupService = serviceProvider.GetRequiredService(); var federatedCredentialService = serviceProvider.GetRequiredService(); @@ -126,7 +125,7 @@ static async Task Main(string[] args) 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(PublishCommand.CreateCommand(publishLogger, configService, agentPublishService, graphApiService, agentBlueprintService, manifestTemplateService)); + rootCommand.AddCommand(PublishCommand.CreateCommand(publishLogger, configService, agentBlueprintService, manifestTemplateService)); // Wrap all command handlers with exception handling // Build with middleware for global exception handling @@ -246,7 +245,6 @@ private static void ConfigureServices(IServiceCollection services, LogLevel mini services.AddSingleton(); services.AddSingleton(); - services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentPublishService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentPublishService.cs deleted file mode 100644 index 44259d8f..00000000 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentPublishService.cs +++ /dev/null @@ -1,343 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -using System.Text; -using System.Text.Json; -using Microsoft.Agents.A365.DevTools.Cli.Constants; -using Microsoft.Extensions.Logging; - -namespace Microsoft.Agents.A365.DevTools.Cli.Services; - -/// -/// Service for agent blueprint publish workflow operations including federated identity credentials, -/// service principal lookups, and app role assignments. -/// -public class AgentPublishService -{ - private readonly ILogger _logger; - private readonly GraphApiService _graphApiService; - - public AgentPublishService(ILogger logger, GraphApiService graphApiService) - { - _logger = logger ?? throw new ArgumentNullException(nameof(logger)); - _graphApiService = graphApiService ?? throw new ArgumentNullException(nameof(graphApiService)); - } - - /// - /// Gets or sets the custom client app ID to use for Microsoft Graph authentication. - /// This delegates to the underlying GraphApiService. - /// - public string? CustomClientAppId - { - get => _graphApiService.CustomClientAppId; - set => _graphApiService.CustomClientAppId = value; - } - - /// - /// Executes the complete publish workflow for an agent blueprint including FIC creation, - /// service principal lookups, and app role assignments. - /// - /// The tenant ID - /// The blueprint application ID - /// The manifest ID for FMI construction - /// Cancellation token - /// True if all steps succeeded, false otherwise - public virtual async Task ExecutePublishGraphStepsAsync( - string tenantId, - string blueprintId, - string manifestId, - CancellationToken cancellationToken = default) - { - try - { - _logger.LogInformation("Configuring agent blueprint for runtime authentication..."); - _logger.LogDebug("TenantId: {TenantId}", tenantId); - _logger.LogDebug("BlueprintId: {BlueprintId}", blueprintId); - _logger.LogDebug("ManifestId: {ManifestId}", manifestId); - - // Step 1: Derive federated identity subject using FMI ID logic - _logger.LogDebug("[STEP 1] Deriving federated identity subject (FMI ID)..."); - - // MOS3 App ID - well-known identifier for MOS (Microsoft Online Services) - const string mos3AppId = "e8be65d6-d430-4289-a665-51bf2a194bda"; - var subjectValue = ConstructFmiId(tenantId, mos3AppId, manifestId); - _logger.LogDebug("Subject value (FMI ID): {Subject}", subjectValue); - - // Step 2: Create federated identity credential - _logger.LogInformation("Configuring workload identity authentication for agent runtime..."); - await CreateFederatedIdentityCredentialAsync( - blueprintId, - subjectValue, - tenantId, - manifestId, - cancellationToken); - - // Step 3: Lookup Service Principal - _logger.LogDebug("[STEP 3] Looking up service principal for blueprint {BlueprintId}...", blueprintId); - var spObjectId = await LookupServicePrincipalAsync(tenantId, blueprintId, cancellationToken); - if (string.IsNullOrWhiteSpace(spObjectId)) - { - _logger.LogError("Failed to lookup service principal for blueprint {BlueprintId}", blueprintId); - _logger.LogError("The agent blueprint service principal may not have been created yet."); - _logger.LogError("Try running 'a365 deploy' or 'a365 setup' to create the agent identity first."); - return false; - } - - _logger.LogDebug("Service principal objectId: {ObjectId}", spObjectId); - - // Step 4: Lookup Microsoft Graph Service Principal - _logger.LogDebug("[STEP 4] Looking up Microsoft Graph service principal..."); - var msGraphResourceId = await LookupMicrosoftGraphServicePrincipalAsync(tenantId, cancellationToken); - if (string.IsNullOrWhiteSpace(msGraphResourceId)) - { - _logger.LogError("Failed to lookup Microsoft Graph service principal"); - return false; - } - - _logger.LogDebug("Microsoft Graph service principal objectId: {ObjectId}", msGraphResourceId); - - // Step 5: Assign app role (optional for agent applications) - _logger.LogInformation("Granting Microsoft Graph permissions to agent blueprint..."); - await AssignAppRoleAsync(tenantId, spObjectId, msGraphResourceId, cancellationToken); - - _logger.LogInformation("Agent blueprint configuration completed successfully"); - return true; - } - catch (Exception ex) - { - _logger.LogError(ex, "Publish graph steps failed: {Message}", ex.Message); - return false; - } - } - - private async Task CreateFederatedIdentityCredentialAsync( - string blueprintId, - string subjectValue, - string tenantId, - string manifestId, - CancellationToken cancellationToken) - { - try - { - var ficName = $"fic-{manifestId}"; - - // Check if FIC already exists - var existing = await _graphApiService.GraphGetAsync(tenantId, $"/beta/applications/{blueprintId}/federatedIdentityCredentials", cancellationToken); - - if (existing != null && existing.RootElement.TryGetProperty("value", out var fics)) - { - foreach (var fic in fics.EnumerateArray()) - { - if (fic.TryGetProperty("subject", out var subject) && - subject.GetString() == subjectValue) - { - _logger.LogInformation("Workload identity authentication already configured"); - return; - } - } - } - - // Create new FIC - var payload = new - { - name = ficName, - issuer = $"https://login.microsoftonline.com/{tenantId}/v2.0", - subject = subjectValue, - audiences = new[] { "api://AzureADTokenExchange" } - }; - - var created = await _graphApiService.GraphPostAsync(tenantId, $"/beta/applications/{blueprintId}/federatedIdentityCredentials", payload, cancellationToken); - - if (created == null) - { - _logger.LogDebug("Failed to create FIC (expected in some scenarios)"); - return; - } - - _logger.LogInformation("Workload identity authentication configured successfully"); - } - catch (Exception ex) - { - _logger.LogError(ex, "Exception creating federated identity credential"); - } - } - - private async Task LookupServicePrincipalAsync( - string tenantId, - string blueprintId, - CancellationToken cancellationToken) - { - try - { - _logger.LogDebug("Looking up service principal for blueprint appId: {BlueprintId}", blueprintId); - var doc = await _graphApiService.GraphGetAsync(tenantId, $"/v1.0/servicePrincipals?$filter=appId eq '{blueprintId}'", cancellationToken); - - if (doc == null) - { - _logger.LogError("Failed to lookup service principal for blueprint {BlueprintId}. Graph API request failed.", blueprintId); - return null; - } - - if (doc.RootElement.TryGetProperty("value", out var value) && value.GetArrayLength() > 0) - { - var sp = value[0]; - if (sp.TryGetProperty("id", out var id)) - { - var spObjectId = id.GetString(); - _logger.LogDebug("Found service principal with objectId: {SpObjectId}", spObjectId); - return spObjectId; - } - } - - _logger.LogWarning("No service principal found for blueprint appId {BlueprintId}. The blueprint's service principal must be created before publish.", blueprintId); - return null; - } - catch (Exception ex) - { - _logger.LogError(ex, "Exception looking up service principal for blueprint {BlueprintId}", blueprintId); - return null; - } - } - - private async Task LookupMicrosoftGraphServicePrincipalAsync( - string tenantId, - CancellationToken cancellationToken) - { - try - { - string msGraphAppId = AuthenticationConstants.MicrosoftGraphResourceAppId; - var doc = await _graphApiService.GraphGetAsync(tenantId, $"/v1.0/servicePrincipals?$filter=appId eq '{msGraphAppId}'&$select=id,appId,displayName", cancellationToken); - - if (doc == null) - { - _logger.LogError("Failed to lookup Microsoft Graph service principal"); - return null; - } - - if (doc.RootElement.TryGetProperty("value", out var value) && value.GetArrayLength() > 0) - { - var sp = value[0]; - if (sp.TryGetProperty("id", out var id)) - { - return id.GetString(); - } - } - - return null; - } - catch (Exception ex) - { - _logger.LogError(ex, "Exception looking up Microsoft Graph service principal"); - return null; - } - } - - private async Task AssignAppRoleAsync( - string tenantId, - string spObjectId, - string msGraphResourceId, - CancellationToken cancellationToken) - { - try - { - // AgentIdUser.ReadWrite.IdentityParentedBy well-known role ID - const string appRoleId = "4aa6e624-eee0-40ab-bdd8-f9639038a614"; - - // Check if role assignment already exists - var existing = await _graphApiService.GraphGetAsync(tenantId, $"/v1.0/servicePrincipals/{spObjectId}/appRoleAssignments", cancellationToken); - - if (existing != null && existing.RootElement.TryGetProperty("value", out var assignments)) - { - foreach (var assignment in assignments.EnumerateArray()) - { - var resourceId = assignment.TryGetProperty("resourceId", out var r) ? r.GetString() : null; - var roleId = assignment.TryGetProperty("appRoleId", out var ar) ? ar.GetString() : null; - - if (resourceId == msGraphResourceId && roleId == appRoleId) - { - _logger.LogInformation("Microsoft Graph permissions already configured"); - return; - } - } - } - - // Create new app role assignment - var payload = new - { - principalId = spObjectId, - resourceId = msGraphResourceId, - appRoleId = appRoleId - }; - - var created = await _graphApiService.GraphPostAsync(tenantId, $"/v1.0/servicePrincipals/{spObjectId}/appRoleAssignments", payload, cancellationToken); - - if (created == null) - { - _logger.LogWarning("Failed to grant Microsoft Graph permissions (continuing anyway)"); - return; - } - - _logger.LogInformation("Microsoft Graph permissions granted successfully"); - } - catch (Exception ex) - { - // Check if this is the known agent application limitation - if (ex.Message.Contains("Service principals of agent applications cannot be set as the source type", StringComparison.OrdinalIgnoreCase)) - { - _logger.LogWarning("App role assignment skipped: Agent applications have restrictions"); - _logger.LogInformation("Agent application permissions should be configured through admin consent URLs"); - return; - } - - _logger.LogWarning(ex, "Exception assigning app role (continuing anyway)"); - } - } - - private static string Base64UrlEncode(byte[] data) - { - if (data == null || data.Length == 0) - { - throw new ArgumentException("Data cannot be null or empty", nameof(data)); - } - - // Convert to Base64 - var base64 = Convert.ToBase64String(data); - - // Make URL-safe: Remove padding and replace characters - return base64.TrimEnd('=') - .Replace('+', '-') - .Replace('/', '_'); - } - - private static string ConstructFmiId(string tenantId, string rmaId, string manifestId) - { - // Parse GUIDs - if (!Guid.TryParse(tenantId, out var tenantGuid)) - { - throw new ArgumentException($"Invalid tenant ID format: {tenantId}", nameof(tenantId)); - } - - if (!Guid.TryParse(rmaId, out var rmaGuid)) - { - throw new ArgumentException($"Invalid RMA/App ID format: {rmaId}", nameof(rmaId)); - } - - // Encode GUIDs as Base64URL - var tenantIdEncoded = Base64UrlEncode(tenantGuid.ToByteArray()); - var rmaIdEncoded = Base64UrlEncode(rmaGuid.ToByteArray()); - - // Construct the FMI namespace - var fmiNamespace = $"/eid1/c/pub/t/{tenantIdEncoded}/a/{rmaIdEncoded}"; - - if (string.IsNullOrWhiteSpace(manifestId)) - { - return fmiNamespace; - } - - // Convert manifestId to Base64URL - this is what MOS will do when impersonating - var manifestIdBytes = Encoding.UTF8.GetBytes(manifestId); - var fmiPath = Base64UrlEncode(manifestIdBytes); - - return $"{fmiNamespace}/{fmiPath}"; - } -} diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/MosTokenService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/MosTokenService.cs deleted file mode 100644 index ac82e18e..00000000 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/MosTokenService.cs +++ /dev/null @@ -1,262 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -using System.Globalization; -using Microsoft.Agents.A365.DevTools.Cli.Constants; -using Microsoft.Agents.A365.DevTools.Cli.Helpers; -using Microsoft.Extensions.Logging; -using Microsoft.Identity.Client; - -namespace Microsoft.Agents.A365.DevTools.Cli.Services; - -/// -/// Native C# service for acquiring MOS (Microsoft Office Store) tokens. -/// Replaces GetToken.ps1 PowerShell script. -/// -public class MosTokenService -{ - private readonly ILogger _logger; - private readonly IConfigService _configService; - private readonly string _cacheFilePath; - - public MosTokenService(ILogger logger, IConfigService configService) - { - _logger = logger; - _configService = configService; - - // Store token cache in user's home directory for security - // Avoid current directory which may have shared/inappropriate permissions - var cacheDir = FileHelper.GetSecureCrossOsDirectory(); - _cacheFilePath = Path.Combine(cacheDir, "mos-token-cache.json"); - } - - /// - /// Acquire MOS token for the specified environment. - /// Uses MSAL.NET for interactive authentication with caching. - /// - public async Task AcquireTokenAsync(string environment, string? personalToken = null, CancellationToken cancellationToken = default) - { - environment = environment.ToLowerInvariant().Trim(); - - // If personal token provided, use it directly (no caching) - if (!string.IsNullOrWhiteSpace(personalToken)) - { - _logger.LogInformation("Using provided personal MOS token override"); - return personalToken.Trim(); - } - - // Try cache first - var cached = TryGetCachedToken(environment); - if (cached.HasValue) - { - _logger.LogInformation("Using cached MOS token (valid until {Expiry:u})", cached.Value.Expiry); - return cached.Value.Token; - } - - // Load config to get tenant ID - var setupConfig = await _configService.LoadAsync(); - if (setupConfig == null) - { - _logger.LogError("Configuration not found. Run 'a365 config init' first."); - return null; - } - - if (string.IsNullOrWhiteSpace(setupConfig.TenantId)) - { - _logger.LogError("TenantId not configured. Run 'a365 config init' first."); - return null; - } - - // Use Microsoft first-party client app for MOS token acquisition - // This is required because MOS APIs only accept tokens from first-party apps - var mosClientAppId = MosConstants.TpsAppServicesClientAppId; - _logger.LogDebug("Using Microsoft first-party client app for MOS tokens: {ClientAppId}", mosClientAppId); - - // Get environment-specific configuration - var config = GetEnvironmentConfig(environment, mosClientAppId, setupConfig.TenantId); - if (config == null) - { - _logger.LogError("Unsupported MOS environment: {Environment}", environment); - return null; - } - - // Acquire new token using MSAL.NET - try - { - _logger.LogInformation("Acquiring MOS token for environment: {Environment}", environment); - _logger.LogInformation("A browser window will open for authentication..."); - - var app = PublicClientApplicationBuilder - .Create(config.ClientId) - .WithAuthority(config.Authority) - .WithRedirectUri(MosConstants.RedirectUri) - .Build(); - - var result = await app - .AcquireTokenInteractive(new[] { config.Scope }) - .WithPrompt(Prompt.SelectAccount) - .ExecuteAsync(cancellationToken); - - if (result?.AccessToken == null) - { - _logger.LogError("Failed to acquire MOS token"); - return null; - } - - // Log the scopes in the token for debugging - if (result.Scopes != null && result.Scopes.Any()) - { - _logger.LogDebug("Token scopes: {Scopes}", string.Join(", ", result.Scopes)); - } - else - { - _logger.LogWarning("Token has no scopes property"); - } - - // Cache the token - var expiry = result.ExpiresOn.UtcDateTime; - CacheToken(environment, result.AccessToken, expiry); - - _logger.LogInformation("MOS token acquired successfully (expires {Expiry:u})", expiry); - return result.AccessToken; - } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to acquire MOS token: {Message}", ex.Message); - return null; - } - } - - private MosEnvironmentConfig? GetEnvironmentConfig(string environment, string clientAppId, string tenantId) - { - // Use tenant-specific authority to support single-tenant apps (AADSTS50194 fix) - var commercialAuthority = $"https://login.microsoftonline.com/{tenantId}"; - var governmentAuthority = $"https://login.microsoftonline.us/{tenantId}"; - - return environment switch - { - "prod" => new MosEnvironmentConfig - { - ClientId = clientAppId, - Authority = commercialAuthority, - Scope = MosConstants.Environments.ProdScope - }, - "sdf" => new MosEnvironmentConfig - { - ClientId = clientAppId, - Authority = commercialAuthority, - Scope = MosConstants.Environments.SdfScope - }, - "test" => new MosEnvironmentConfig - { - ClientId = clientAppId, - Authority = commercialAuthority, - Scope = MosConstants.Environments.TestScope - }, - "gccm" => new MosEnvironmentConfig - { - ClientId = clientAppId, - Authority = commercialAuthority, - Scope = MosConstants.Environments.GccmScope - }, - "gcch" => new MosEnvironmentConfig - { - ClientId = clientAppId, - Authority = governmentAuthority, - Scope = MosConstants.Environments.GcchScope - }, - "dod" => new MosEnvironmentConfig - { - ClientId = clientAppId, - Authority = governmentAuthority, - Scope = MosConstants.Environments.DodScope - }, - _ => null - }; - } - - private (string Token, DateTime Expiry)? TryGetCachedToken(string environment) - { - try - { - if (!File.Exists(_cacheFilePath)) - return null; - - var json = File.ReadAllText(_cacheFilePath); - using var doc = System.Text.Json.JsonDocument.Parse(json); - - if (doc.RootElement.TryGetProperty(environment, out var envElement)) - { - var token = envElement.TryGetProperty("token", out var t) ? t.GetString() : null; - var expiryStr = envElement.TryGetProperty("expiry", out var e) ? e.GetString() : null; - - if (!string.IsNullOrWhiteSpace(token) && DateTime.TryParse(expiryStr, CultureInfo.InvariantCulture, DateTimeStyles.AdjustToUniversal, out var expiry)) - { - // Return cached token if valid for at least 2 more minutes - if (DateTime.UtcNow < expiry.AddMinutes(-2)) - { - return (token, expiry); - } - } - } - - return null; - } - catch (Exception ex) - { - _logger.LogDebug(ex, "Failed to read token cache"); - return null; - } - } - - private void CacheToken(string environment, string token, DateTime expiry) - { - try - { - var cache = new Dictionary(); - - if (File.Exists(_cacheFilePath)) - { - var json = File.ReadAllText(_cacheFilePath); - cache = System.Text.Json.JsonSerializer.Deserialize>(json) ?? new(); - } - - cache[environment] = new - { - token, - expiry = expiry.ToUniversalTime().ToString("o") - }; - - var updated = System.Text.Json.JsonSerializer.Serialize(cache, new System.Text.Json.JsonSerializerOptions { WriteIndented = true }); - File.WriteAllText(_cacheFilePath, updated); - - // Set file permissions to user-only on Unix systems - if (!OperatingSystem.IsWindows()) - { - try - { - var fileInfo = new FileInfo(_cacheFilePath); - fileInfo.UnixFileMode = UnixFileMode.UserRead | UnixFileMode.UserWrite; - _logger.LogDebug("Set secure permissions (0600) on token cache file"); - } - catch (Exception permEx) - { - _logger.LogWarning(permEx, "Failed to set Unix file permissions on token cache"); - } - } - - _logger.LogDebug("Token cached for environment: {Environment} at {Path}", environment, _cacheFilePath); - } - catch (Exception ex) - { - _logger.LogWarning(ex, "Failed to cache token"); - } - } - - private class MosEnvironmentConfig - { - public required string ClientId { get; init; } - public required string Authority { get; init; } - public required string Scope { get; init; } - } -} 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..14561ed6 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 @@ -21,16 +21,13 @@ public class PublishCommandTestCollection { } /// /// Tests for PublishCommand exit code behavior. /// Tests are limited to paths that exit before the interactive Console.ReadLine() prompts -/// in the publish flow. Paths that reach those prompts (--skip-graph, missing tenantId, -/// missing manifest file) require full HTTP/MOS mocking infrastructure to test reliably. +/// in the publish flow. /// [Collection("PublishCommandTests")] public class PublishCommandTests : IDisposable { private readonly ILogger _logger; private readonly IConfigService _configService; - private readonly AgentPublishService _agentPublishService; - private readonly GraphApiService _graphApiService; private readonly AgentBlueprintService _blueprintService; private readonly ManifestTemplateService _manifestTemplateService; private readonly TextReader _originalConsoleIn = Console.In; @@ -40,19 +37,12 @@ public PublishCommandTests() _logger = Substitute.For>(); _configService = Substitute.For(); - // For concrete classes, create partial substitutes with correct constructor parameters - // GraphApiService has a parameterless constructor - _graphApiService = Substitute.ForPartsOf(); - - // AgentPublishService needs (ILogger, GraphApiService) - _agentPublishService = Substitute.ForPartsOf( - Substitute.For>(), - _graphApiService); + var graphApiService = Substitute.ForPartsOf(); // AgentBlueprintService needs (ILogger, GraphApiService) _blueprintService = Substitute.ForPartsOf( Substitute.For>(), - _graphApiService); + graphApiService); // ManifestTemplateService needs only ILogger _manifestTemplateService = Substitute.ForPartsOf( @@ -81,8 +71,6 @@ public async Task PublishCommand_WithMissingBlueprintId_ShouldReturnExitCode1() var command = PublishCommand.CreateCommand( _logger, _configService, - _agentPublishService, - _graphApiService, _blueprintService, _manifestTemplateService); @@ -132,8 +120,6 @@ public async Task PublishCommand_WithDryRun_ShouldReturnExitCode0() var command = PublishCommand.CreateCommand( _logger, _configService, - _agentPublishService, - _graphApiService, _blueprintService, _manifestTemplateService); @@ -163,6 +149,58 @@ public async Task PublishCommand_WithDryRun_ShouldReturnExitCode0() } } + [Fact] + public async Task PublishCommand_WithValidConfig_CreatesZipAndReturnsExitCode0() + { + // Arrange + var tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); + var manifestDir = Path.Combine(tempDir, "manifest"); + Directory.CreateDirectory(manifestDir); + + try + { + var manifestPath = Path.Combine(manifestDir, "manifest.json"); + var agenticUserManifestPath = Path.Combine(manifestDir, "agenticUserTemplateManifest.json"); + await File.WriteAllTextAsync(manifestPath, "{\"id\":\"old-id\"}"); + await File.WriteAllTextAsync(agenticUserManifestPath, "{\"agentIdentityBlueprintId\":\"old-id\"}"); + + var config = new Agent365Config + { + AgentBlueprintId = "test-blueprint-id", + AgentBlueprintDisplayName = "Test Agent", + TenantId = "test-tenant", + DeploymentProjectPath = tempDir + }; + _configService.LoadAsync().Returns(config); + + // Redirect stdin so interactive prompts auto-answer + Console.SetIn(new StringReader("n\n\n")); + + var command = PublishCommand.CreateCommand( + _logger, + _configService, + _blueprintService, + _manifestTemplateService); + + var root = new RootCommand(); + root.AddCommand(command); + + // Act + var exitCode = await root.InvokeAsync("publish"); + + // Assert + exitCode.Should().Be(0, "successful zip creation should return exit code 0"); + File.Exists(Path.Combine(manifestDir, "manifest.zip")).Should().BeTrue("manifest.zip should be created"); + } + finally + { + if (Directory.Exists(tempDir)) + { + Directory.Delete(tempDir, true); + } + } + } + [Fact] public async Task PublishCommand_WithException_ShouldReturnExitCode1() { @@ -173,8 +211,6 @@ public async Task PublishCommand_WithException_ShouldReturnExitCode1() var command = PublishCommand.CreateCommand( _logger, _configService, - _agentPublishService, - _graphApiService, _blueprintService, _manifestTemplateService); @@ -197,7 +233,7 @@ public async Task PublishCommand_WithException_ShouldReturnExitCode1() } /// - /// Documents the four normal exit scenarios (exit code 0) and the main error scenarios (exit code 1). + /// Documents the normal exit scenarios (exit code 0) and the main error scenarios (exit code 1). /// [Fact] public void PublishCommand_DocumentsNormalExitScenarios() @@ -205,9 +241,7 @@ public void PublishCommand_DocumentsNormalExitScenarios() var normalExitScenarios = new[] { "Dry-run: --dry-run specified, manifest updated but not saved", - "Skip Graph: --skip-graph specified, MOS publish succeeded", - "Missing tenantId: MOS publish succeeded but tenantId unavailable for Graph operations", - "Complete success: MOS publish and Graph operations both succeeded" + "Complete success: manifest updated and manifest.zip created" }; var errorExitScenarios = new[] @@ -215,12 +249,11 @@ public void PublishCommand_DocumentsNormalExitScenarios() "Missing blueprintId in configuration", "Failed to extract manifest templates", "Manifest file not found", - "MOS API call failed", - "Graph API operations failed", + "No manifest files found to zip", "Exception thrown during execution" }; - normalExitScenarios.Should().HaveCount(4, "there are exactly 4 normal exit scenarios"); - errorExitScenarios.Length.Should().BeGreaterThan(5, "there are many error exit scenarios"); + normalExitScenarios.Should().HaveCount(2, "there are exactly 2 normal exit scenarios"); + errorExitScenarios.Length.Should().BeGreaterThan(3, "there are multiple error exit scenarios"); } } diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishHelpersTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishHelpersTests.cs deleted file mode 100644 index b3222f4a..00000000 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishHelpersTests.cs +++ /dev/null @@ -1,465 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -using FluentAssertions; -using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Logging.Abstractions; -using Microsoft.Agents.A365.DevTools.Cli.Services; -using Microsoft.Agents.A365.DevTools.Cli.Models; -using Microsoft.Agents.A365.DevTools.Cli.Helpers; -using Microsoft.Agents.A365.DevTools.Cli.Exceptions; -using Microsoft.Agents.A365.DevTools.Cli.Constants; -using Moq; -using System.Text.Json; - -namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Commands; - -/// -/// Unit tests for MOS prerequisites in PublishHelpers -/// -public class PublishHelpersTests -{ - private readonly Mock _mockLogger; - private readonly Mock _mockGraphService; - private readonly Mock _mockBlueprintService; - private readonly Agent365Config _testConfig; - - public PublishHelpersTests() - { - _mockLogger = new Mock(); - - // Create GraphApiService with all mocked dependencies to prevent real API calls - // This matches the pattern used in GraphApiServiceTests - var mockGraphLogger = new Mock>(); - var mockExecutor = new Mock(MockBehavior.Loose, NullLogger.Instance); - var mockTokenProvider = new Mock(); - - // Create mock using constructor with all dependencies to prevent real HTTP/Auth calls - _mockGraphService = new Mock( - mockGraphLogger.Object, - mockExecutor.Object, - It.IsAny(), - mockTokenProvider.Object) - { - CallBase = false - }; - - // Create AgentBlueprintService mock - var mockBlueprintLogger = new Mock>(); - _mockBlueprintService = new Mock( - mockBlueprintLogger.Object, - _mockGraphService.Object) - { - CallBase = false - }; - - _testConfig = new Agent365Config - { - TenantId = "test-tenant-id", - ClientAppId = "test-client-app-id" - }; - } - - [Fact] - public async Task EnsureMosPrerequisitesAsync_WhenClientAppIdMissing_ThrowsSetupValidationException() - { - // Arrange - var config = new Agent365Config { ClientAppId = "" }; - - // Act - Func act = async () => await PublishHelpers.EnsureMosPrerequisitesAsync( - _mockGraphService.Object, _mockBlueprintService.Object, config, _mockLogger.Object); - - // Assert - await act.Should().ThrowAsync() - .WithMessage("*Custom client app ID is required*"); - } - - [Fact] - public async Task EnsureMosPrerequisitesAsync_WhenCustomAppNotFound_ThrowsSetupValidationException() - { - // Arrange - var emptyAppsResponse = JsonDocument.Parse("{\"value\": []}"); - _mockGraphService.Setup(x => x.GraphGetAsync( - It.IsAny(), - It.Is(s => s.Contains($"appId eq '{_testConfig.ClientAppId}'")), - It.IsAny(), - It.IsAny?>())) - .ReturnsAsync(emptyAppsResponse); - - // Act - Func act = async () => await PublishHelpers.EnsureMosPrerequisitesAsync( - _mockGraphService.Object, _mockBlueprintService.Object, _testConfig, _mockLogger.Object); - - // Assert - await act.Should().ThrowAsync() - .WithMessage("*not found*"); - } - - [Fact] - public async Task EnsureMosPrerequisitesAsync_WhenPermissionsAlreadyExist_ReturnsTrue() - { - // Arrange - app with ALL MOS permissions correctly configured - var appWithMosPermissions = JsonDocument.Parse($@"{{ - ""value"": [{{ - ""id"": ""app-object-id"", - ""requiredResourceAccess"": [ - {{ - ""resourceAppId"": ""{MosConstants.TpsAppServicesResourceAppId}"", - ""resourceAccess"": [{{ ""id"": ""{MosConstants.ResourcePermissions.TpsAppServices.ScopeId}"", ""type"": ""Scope"" }}] - }}, - {{ - ""resourceAppId"": ""{MosConstants.PowerPlatformApiResourceAppId}"", - ""resourceAccess"": [{{ ""id"": ""{MosConstants.ResourcePermissions.PowerPlatformApi.ScopeId}"", ""type"": ""Scope"" }}] - }}, - {{ - ""resourceAppId"": ""{MosConstants.MosTitlesApiResourceAppId}"", - ""resourceAccess"": [{{ ""id"": ""{MosConstants.ResourcePermissions.MosTitlesApi.ScopeId}"", ""type"": ""Scope"" }}] - }} - ] - }}] - }}"); - - var tpsConsentGrantDoc = JsonDocument.Parse(@"{ - ""value"": [{ - ""scope"": ""AuthConfig.Read"" - }] - }"); - - var powerPlatformConsentGrantDoc = JsonDocument.Parse(@"{ - ""value"": [{ - ""scope"": ""EnvironmentManagement.Environments.Read"" - }] - }"); - - var mosTitlesConsentGrantDoc = JsonDocument.Parse(@"{ - ""value"": [{ - ""scope"": ""Title.ReadWrite.All"" - }] - }"); - - _mockGraphService.Setup(x => x.GraphGetAsync( - It.IsAny(), - It.Is(s => s.Contains($"appId eq '{_testConfig.ClientAppId}'")), - It.IsAny(), - It.IsAny?>())) - .ReturnsAsync(appWithMosPermissions); - - // Mock consent grants - since all SP lookups return "sp-object-id", - // the consent grant query filter will always contain "sp-object-id" for both client and resource - // We need to mock based on the actual filter pattern used in CheckMosPrerequisitesAsync - _mockGraphService.Setup(x => x.GraphGetAsync( - It.IsAny(), - It.Is(s => s.Contains("oauth2PermissionGrants")), - It.IsAny(), - It.IsAny?>())) - .ReturnsAsync((string tenant, string path, CancellationToken ct, IEnumerable? headers) => - { - // Return appropriate consent based on which check is being done - // Since we can't differentiate between resources (all return same SP ID), - // return a combined consent that satisfies all checks - return JsonDocument.Parse(@"{ - ""value"": [{ - ""scope"": ""AuthConfig.Read EnvironmentManagement.Environments.Read Title.ReadWrite.All"" - }] - }"); - }); - - _mockGraphService.Setup(x => x.LookupServicePrincipalByAppIdAsync( - It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny?>())) - .ReturnsAsync("sp-object-id"); - - _mockGraphService.Setup(x => x.GraphPatchAsync( - It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny?>())) - .ReturnsAsync(true); - - // Act - var result = await PublishHelpers.EnsureMosPrerequisitesAsync( - _mockGraphService.Object, _mockBlueprintService.Object, _testConfig, _mockLogger.Object); - - // Assert - result.Should().BeTrue(); - - // When all prerequisites exist, EnsureServicePrincipalForAppIdAsync should NOT be called - _mockGraphService.Verify(x => x.EnsureServicePrincipalForAppIdAsync( - It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny?>()), - Times.Never()); - - // Should verify all service principals exist via lookup - _mockGraphService.Verify(x => x.LookupServicePrincipalByAppIdAsync( - It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny?>()), - Times.AtLeast(1 + MosConstants.AllResourceAppIds.Length)); - } - - [Fact] - public async Task EnsureMosPrerequisitesAsync_WhenPermissionsMissing_CreatesServicePrincipals() - { - // Arrange - app with NO MOS permissions - var appWithoutMosPermissions = JsonDocument.Parse(@"{ - ""value"": [{ - ""id"": ""app-object-id"", - ""requiredResourceAccess"": [] - }] - }"); - - var emptyConsentDoc = JsonDocument.Parse(@"{ ""value"": [] }"); - - _mockGraphService.Setup(x => x.GraphGetAsync( - It.IsAny(), - It.Is(s => s.Contains($"appId eq '{_testConfig.ClientAppId}'")), - It.IsAny(), - It.IsAny?>())) - .ReturnsAsync(appWithoutMosPermissions); - - _mockGraphService.Setup(x => x.GraphGetAsync( - It.IsAny(), - It.Is(s => s.Contains("oauth2PermissionGrants")), - It.IsAny(), - It.IsAny?>())) - .ReturnsAsync(emptyConsentDoc); - - // Service principals don't exist initially (return null), then exist after creation (return ID) - // Track which SPs have been created - var createdSps = new HashSet(); - _mockGraphService.Setup(x => x.LookupServicePrincipalByAppIdAsync( - It.IsAny(), It.Is(appId => appId == MosConstants.TpsAppServicesClientAppId), It.IsAny(), It.IsAny?>())) - .ReturnsAsync(() => createdSps.Contains(MosConstants.TpsAppServicesClientAppId) ? "sp-object-id" : null); - - foreach (var resourceAppId in MosConstants.AllResourceAppIds) - { - var capturedAppId = resourceAppId; // Capture for closure - _mockGraphService.Setup(x => x.LookupServicePrincipalByAppIdAsync( - It.IsAny(), It.Is(appId => appId == capturedAppId), It.IsAny(), It.IsAny?>())) - .ReturnsAsync(() => createdSps.Contains(capturedAppId) ? "sp-object-id" : null); - } - - _mockGraphService.Setup(x => x.EnsureServicePrincipalForAppIdAsync( - It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny?>())) - .ReturnsAsync((string tenantId, string appId, CancellationToken ct, IEnumerable? authScopes) => - { - createdSps.Add(appId); // Mark as created - return "sp-object-id"; - }); - - _mockGraphService.Setup(x => x.GraphPatchAsync( - It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny?>())) - .ReturnsAsync(true); - - _mockBlueprintService.Setup(x => x.ReplaceOauth2PermissionGrantAsync( - It.IsAny(), - It.IsAny(), - It.IsAny(), - It.IsAny>(), - default)) - .ReturnsAsync(true); - - // Act - var result = await PublishHelpers.EnsureMosPrerequisitesAsync( - _mockGraphService.Object, _mockBlueprintService.Object, _testConfig, _mockLogger.Object); - - // Assert - result.Should().BeTrue(); - - // Should create service principals for first-party client app + MOS resource apps - var expectedServicePrincipalCalls = 1 + MosConstants.AllResourceAppIds.Length; - _mockGraphService.Verify(x => x.EnsureServicePrincipalForAppIdAsync( - It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny?>()), - Times.Exactly(expectedServicePrincipalCalls)); - } - - [Fact] - public async Task EnsureMosPrerequisitesAsync_WhenServicePrincipalCreationFails_ThrowsSetupValidationException() - { - // Arrange - var appWithoutMosPermissions = JsonDocument.Parse(@"{ - ""value"": [{ - ""id"": ""app-object-id"", - ""requiredResourceAccess"": [] - }] - }"); - - _mockGraphService.Setup(x => x.GraphGetAsync( - It.IsAny(), - It.Is(s => s.Contains($"appId eq '{_testConfig.ClientAppId}'")), - It.IsAny(), - It.IsAny?>())) - .ReturnsAsync(appWithoutMosPermissions); - - _mockGraphService.Setup(x => x.CheckServicePrincipalCreationPrivilegesAsync( - It.IsAny(), It.IsAny())) - .ReturnsAsync((true, new List { "Application Administrator" })); - - _mockGraphService.Setup(x => x.EnsureServicePrincipalForAppIdAsync( - It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny?>())) - .ThrowsAsync(new InvalidOperationException("Failed to create service principal")); - - // Act - Func act = async () => await PublishHelpers.EnsureMosPrerequisitesAsync( - _mockGraphService.Object, _mockBlueprintService.Object, _testConfig, _mockLogger.Object); - - // Assert - await act.Should().ThrowAsync() - .WithMessage("*Failed to create service principal*"); - } - - [Fact] - public async Task EnsureMosPrerequisitesAsync_WhenInsufficientPrivileges_ThrowsWithAzCliGuidance() - { - // Arrange - var appWithoutMosPermissions = JsonDocument.Parse(@"{ - ""value"": [{ - ""id"": ""app-object-id"", - ""requiredResourceAccess"": [] - }] - }"); - - _mockGraphService.Setup(x => x.GraphGetAsync( - It.IsAny(), - It.Is(s => s.Contains($"appId eq '{_testConfig.ClientAppId}'")), - It.IsAny(), - It.IsAny?>())) - .ReturnsAsync(appWithoutMosPermissions); - - _mockGraphService.Setup(x => x.CheckServicePrincipalCreationPrivilegesAsync( - It.IsAny(), It.IsAny())) - .ReturnsAsync((false, new List())); - - _mockGraphService.Setup(x => x.EnsureServicePrincipalForAppIdAsync( - It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny?>())) - .ThrowsAsync(new Exception("403 Forbidden")); - - // Act - Func act = async () => await PublishHelpers.EnsureMosPrerequisitesAsync( - _mockGraphService.Object, _mockBlueprintService.Object, _testConfig, _mockLogger.Object); - - // Assert - await act.Should().ThrowAsync() - .WithMessage("*Insufficient privileges*"); - } - - [Fact] - public async Task EnsureMosPrerequisitesAsync_WhenCalledTwice_IsIdempotent() - { - // Arrange - app with ALL MOS permissions and consent correctly configured - var appWithMosPermissions = JsonDocument.Parse($@"{{ - ""value"": [{{ - ""id"": ""app-object-id"", - ""requiredResourceAccess"": [ - {{ - ""resourceAppId"": ""{MosConstants.TpsAppServicesResourceAppId}"", - ""resourceAccess"": [{{ ""id"": ""{MosConstants.ResourcePermissions.TpsAppServices.ScopeId}"", ""type"": ""Scope"" }}] - }}, - {{ - ""resourceAppId"": ""{MosConstants.PowerPlatformApiResourceAppId}"", - ""resourceAccess"": [{{ ""id"": ""{MosConstants.ResourcePermissions.PowerPlatformApi.ScopeId}"", ""type"": ""Scope"" }}] - }}, - {{ - ""resourceAppId"": ""{MosConstants.MosTitlesApiResourceAppId}"", - ""resourceAccess"": [{{ ""id"": ""{MosConstants.ResourcePermissions.MosTitlesApi.ScopeId}"", ""type"": ""Scope"" }}] - }} - ] - }}] - }}"); - - // Mock consent grants for each MOS resource app with correct scopes - var tpsConsentDoc = JsonDocument.Parse($@"{{ - ""value"": [{{ - ""scope"": ""{MosConstants.ResourcePermissions.TpsAppServices.ScopeName}"" - }}] - }}"); - - var ppConsentDoc = JsonDocument.Parse($@"{{ - ""value"": [{{ - ""scope"": ""{MosConstants.ResourcePermissions.PowerPlatformApi.ScopeName}"" - }}] - }}"); - - var titlesConsentDoc = JsonDocument.Parse($@"{{ - ""value"": [{{ - ""scope"": ""{MosConstants.ResourcePermissions.MosTitlesApi.ScopeName}"" - }}] - }}"); - - _mockGraphService.Setup(x => x.GraphGetAsync( - It.IsAny(), - It.Is(s => s.Contains($"appId eq '{_testConfig.ClientAppId}'")), - It.IsAny(), - It.IsAny?>())) - .ReturnsAsync(appWithMosPermissions); - - // Mock consent grants based on resourceId (SP object ID) in the query filter - _mockGraphService.Setup(x => x.GraphGetAsync( - It.IsAny(), - It.Is(s => s.Contains("oauth2PermissionGrants") && s.Contains("sp-tps")), - It.IsAny(), - It.IsAny?>())) - .ReturnsAsync(tpsConsentDoc); - - _mockGraphService.Setup(x => x.GraphGetAsync( - It.IsAny(), - It.Is(s => s.Contains("oauth2PermissionGrants") && s.Contains("sp-pp")), - It.IsAny(), - It.IsAny?>())) - .ReturnsAsync(ppConsentDoc); - - _mockGraphService.Setup(x => x.GraphGetAsync( - It.IsAny(), - It.Is(s => s.Contains("oauth2PermissionGrants") && s.Contains("sp-titles")), - It.IsAny(), - It.IsAny?>())) - .ReturnsAsync(titlesConsentDoc); - - // Mock service principal lookups - return unique IDs for each resource app - _mockGraphService.Setup(x => x.LookupServicePrincipalByAppIdAsync( - It.IsAny(), MosConstants.TpsAppServicesClientAppId, It.IsAny(), It.IsAny?>())) - .ReturnsAsync("sp-first-party-client"); - - _mockGraphService.Setup(x => x.LookupServicePrincipalByAppIdAsync( - It.IsAny(), MosConstants.TpsAppServicesResourceAppId, It.IsAny(), It.IsAny?>())) - .ReturnsAsync("sp-tps"); - - _mockGraphService.Setup(x => x.LookupServicePrincipalByAppIdAsync( - It.IsAny(), MosConstants.PowerPlatformApiResourceAppId, It.IsAny(), It.IsAny?>())) - .ReturnsAsync("sp-pp"); - - _mockGraphService.Setup(x => x.LookupServicePrincipalByAppIdAsync( - It.IsAny(), MosConstants.MosTitlesApiResourceAppId, It.IsAny(), It.IsAny?>())) - .ReturnsAsync("sp-titles"); - - _mockGraphService.Setup(x => x.GraphPatchAsync( - It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny?>())) - .ReturnsAsync(true); - - // Act - var result1 = await PublishHelpers.EnsureMosPrerequisitesAsync( - _mockGraphService.Object, _mockBlueprintService.Object, _testConfig, _mockLogger.Object); - var result2 = await PublishHelpers.EnsureMosPrerequisitesAsync( - _mockGraphService.Object, _mockBlueprintService.Object, _testConfig, _mockLogger.Object); - - // Assert - result1.Should().BeTrue(); - result2.Should().BeTrue(); - - // Should query the app once per call - _mockGraphService.Verify(x => x.GraphGetAsync( - It.IsAny(), - It.Is(s => s.Contains($"appId eq '{_testConfig.ClientAppId}'")), - It.IsAny(), - It.IsAny?>()), Times.Exactly(2)); - - // When all prerequisites exist, EnsureServicePrincipalForAppIdAsync should NEVER be called (truly idempotent) - _mockGraphService.Verify(x => x.EnsureServicePrincipalForAppIdAsync( - It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny?>()), - Times.Never()); - - // GraphPatchAsync should never be called since permissions are already correct - _mockGraphService.Verify(x => x.GraphPatchAsync( - It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny?>()), - Times.Never()); - - // ReplaceOauth2PermissionGrantAsync should never be called since consent already exists - _mockBlueprintService.Verify(x => x.ReplaceOauth2PermissionGrantAsync( - It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>(), It.IsAny()), - Times.Never()); - } -} diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentPublishServiceTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentPublishServiceTests.cs deleted file mode 100644 index 22ac9181..00000000 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentPublishServiceTests.cs +++ /dev/null @@ -1,231 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -using System.Net; -using System.Net.Http; -using System.Text.Json; -using FluentAssertions; -using Microsoft.Agents.A365.DevTools.Cli.Constants; -using Microsoft.Agents.A365.DevTools.Cli.Services; -using Microsoft.Extensions.Logging; -using NSubstitute; -using Xunit; - -namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Services; - -public class AgentPublishServiceTests -{ - private readonly ILogger _mockLogger; - private readonly ILogger _mockGraphLogger; - private readonly CommandExecutor _mockExecutor; - - public AgentPublishServiceTests() - { - _mockLogger = Substitute.For>(); - _mockGraphLogger = Substitute.For>(); - var mockExecutorLogger = Substitute.For>(); - _mockExecutor = Substitute.ForPartsOf(mockExecutorLogger); - } - - [Fact] - public async Task ExecutePublishGraphStepsAsync_MakesExpectedHttpCallsInCorrectOrder() - { - // CRITICAL INTEGRATION TEST: This test captures the EXACT behavior of ExecutePublishGraphStepsAsync - // before refactoring. It validates: - // 1. The sequence of HTTP calls (GET/POST order) - // 2. The URLs and query parameters - // 3. Header values (Authorization present, ConsistencyLevel ABSENT) - // 4. Request payloads - // 5. Idempotency checks (checking before creating) - // - // This test serves as a REGRESSION GUARD during refactoring. After refactoring private methods - // to use GraphGetAsync/GraphPostAsync helpers, this test MUST still pass, proving behavior is unchanged. - // - // Test approach: Use CapturingHttpMessageHandler to record ALL HTTP requests, then validate - // each request matches expected behavior. - - // Arrange - var capturedRequests = new List(); - var handler = new MultiCapturingHttpMessageHandler((req) => capturedRequests.Add(req)); - var executor = Substitute.For(Substitute.For>()); - - const string tenantId = "11111111-1111-1111-1111-111111111111"; - const string blueprintId = "22222222-2222-2222-2222-222222222222"; - const string manifestId = "test-manifest-id"; - const string spObjectId = "sp-33333333-3333-3333-3333-333333333333"; - const string msGraphSpId = "ms-44444444-4444-4444-4444-444444444444"; - - // Mock az CLI token acquisition - executor.ExecuteAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) - .Returns(callInfo => - { - var cmd = callInfo.ArgAt(0); - var args = callInfo.ArgAt(1); - - if (cmd == "az" && args != null && args.StartsWith("account show", StringComparison.OrdinalIgnoreCase)) - { - return Task.FromResult(new CommandResult - { - ExitCode = 0, - StandardOutput = JsonSerializer.Serialize(new { tenantId }), - StandardError = string.Empty - }); - } - - if (cmd == "az" && args != null && args.Contains("get-access-token", StringComparison.OrdinalIgnoreCase)) - { - return Task.FromResult(new CommandResult - { - ExitCode = 0, - StandardOutput = "test-bearer-token-xyz", - StandardError = string.Empty - }); - } - - return Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = string.Empty, StandardError = string.Empty }); - }); - - var graphService = new GraphApiService(_mockGraphLogger, executor, handler); - var service = new AgentPublishService(_mockLogger, graphService); - - // Expected HTTP call sequence (based on current implementation): - // 1. GET /beta/applications/{blueprintId}/federatedIdentityCredentials - check if FIC exists - // 2. POST /beta/applications/{blueprintId}/federatedIdentityCredentials - create FIC (if not exists) - // 3. GET /v1.0/servicePrincipals?$filter=appId eq '{blueprintId}' - lookup SP - // 4. GET /v1.0/servicePrincipals?$filter=appId eq '{msGraphAppId}' - lookup MS Graph SP - // 5. GET /v1.0/servicePrincipals/{spObjectId}/appRoleAssignments - check if role exists - // 6. POST /v1.0/servicePrincipals/{spObjectId}/appRoleAssignments - assign role (if not exists) - - // Queue responses for each expected HTTP call - // Response 1: GET FIC - return empty (FIC doesn't exist) - handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) - { - Content = new StringContent(JsonSerializer.Serialize(new { value = Array.Empty() })) - }); - - // Response 2: POST FIC - return created - handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.Created) - { - Content = new StringContent(JsonSerializer.Serialize(new { id = "fic-created-id", name = $"fic-{manifestId}" })) - }); - - // Response 3: GET SP by appId - return service principal - handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) - { - Content = new StringContent(JsonSerializer.Serialize(new { value = new[] { new { id = spObjectId, appId = blueprintId } } })) - }); - - // Response 4: GET MS Graph SP - return Microsoft Graph service principal - handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) - { - Content = new StringContent(JsonSerializer.Serialize(new { value = new[] { new { id = msGraphSpId, appId = AuthenticationConstants.MicrosoftGraphResourceAppId } } })) - }); - - // Response 5: GET app role assignments - return empty (role doesn't exist) - handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) - { - Content = new StringContent(JsonSerializer.Serialize(new { value = Array.Empty() })) - }); - - // Response 6: POST app role assignment - return created - handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.Created) - { - Content = new StringContent(JsonSerializer.Serialize(new { id = "role-assignment-id" })) - }); - - // Act - var result = await service.ExecutePublishGraphStepsAsync(tenantId, blueprintId, manifestId, CancellationToken.None); - - // Assert - result.Should().BeTrue("ExecutePublishGraphStepsAsync should succeed"); - capturedRequests.Should().HaveCount(6, "should make exactly 6 HTTP calls"); - - // Validate Request 1: GET federated identity credentials - var req1 = capturedRequests[0]; - req1.Method.Should().Be(HttpMethod.Get); - req1.RequestUri.Should().NotBeNull(); - req1.RequestUri!.AbsolutePath.Should().Be($"/beta/applications/{blueprintId}/federatedIdentityCredentials"); - req1.Headers.Authorization.Should().NotBeNull(); - req1.Headers.Authorization!.Scheme.Should().Be("Bearer"); - req1.Headers.Authorization.Parameter.Should().Be("test-bearer-token-xyz"); - req1.Headers.Contains("ConsistencyLevel").Should().BeFalse("ConsistencyLevel header must NOT be present"); - - // Validate Request 2: POST federated identity credential - var req2 = capturedRequests[1]; - req2.Method.Should().Be(HttpMethod.Post); - req2.RequestUri.Should().NotBeNull(); - req2.RequestUri!.AbsolutePath.Should().Be($"/beta/applications/{blueprintId}/federatedIdentityCredentials"); - req2.Headers.Authorization.Should().NotBeNull(); - req2.Headers.Contains("ConsistencyLevel").Should().BeFalse("ConsistencyLevel header must NOT be present"); - req2.Content.Should().NotBeNull(); - var req2Body = await req2.Content!.ReadAsStringAsync(); - req2Body.Should().Contain($"fic-{manifestId}"); - req2Body.Should().Contain($"https://login.microsoftonline.com/{tenantId}/v2.0"); - req2Body.Should().Contain("api://AzureADTokenExchange"); - - // Validate Request 3: GET service principal by appId - var req3 = capturedRequests[2]; - req3.Method.Should().Be(HttpMethod.Get); - req3.RequestUri.Should().NotBeNull(); - req3.RequestUri!.AbsolutePath.Should().Be("/v1.0/servicePrincipals"); - Uri.UnescapeDataString(req3.RequestUri.Query).Should().Contain($"$filter=appId eq '{blueprintId}'"); - req3.Headers.Authorization.Should().NotBeNull(); - req3.Headers.Contains("ConsistencyLevel").Should().BeFalse("ConsistencyLevel header must NOT be present"); - - // Validate Request 4: GET Microsoft Graph service principal - var req4 = capturedRequests[3]; - req4.Method.Should().Be(HttpMethod.Get); - req4.RequestUri.Should().NotBeNull(); - req4.RequestUri!.AbsolutePath.Should().Be("/v1.0/servicePrincipals"); - Uri.UnescapeDataString(req4.RequestUri.Query).Should().Contain($"$filter=appId eq '{AuthenticationConstants.MicrosoftGraphResourceAppId}'"); - req4.Headers.Authorization.Should().NotBeNull(); - req4.Headers.Contains("ConsistencyLevel").Should().BeFalse("ConsistencyLevel header must NOT be present"); - - // Validate Request 5: GET app role assignments - var req5 = capturedRequests[4]; - req5.Method.Should().Be(HttpMethod.Get); - req5.RequestUri.Should().NotBeNull(); - req5.RequestUri!.AbsolutePath.Should().Be($"/v1.0/servicePrincipals/{spObjectId}/appRoleAssignments"); - req5.Headers.Authorization.Should().NotBeNull(); - req5.Headers.Contains("ConsistencyLevel").Should().BeFalse("ConsistencyLevel header must NOT be present"); - - // Validate Request 6: POST app role assignment - var req6 = capturedRequests[5]; - req6.Method.Should().Be(HttpMethod.Post); - req6.RequestUri.Should().NotBeNull(); - req6.RequestUri!.AbsolutePath.Should().Be($"/v1.0/servicePrincipals/{spObjectId}/appRoleAssignments"); - req6.Headers.Authorization.Should().NotBeNull(); - req6.Headers.Contains("ConsistencyLevel").Should().BeFalse("ConsistencyLevel header must NOT be present"); - req6.Content.Should().NotBeNull(); - var req6Body = await req6.Content!.ReadAsStringAsync(); - req6Body.Should().Contain(spObjectId); - req6Body.Should().Contain(msGraphSpId); - req6Body.Should().Contain("4aa6e624-eee0-40ab-bdd8-f9639038a614"); // AgentIdUser.ReadWrite.IdentityParentedBy role ID - } -} - -// Multi-capturing handler that captures ALL requests in a list (for integration tests) -internal class MultiCapturingHttpMessageHandler : HttpMessageHandler -{ - private readonly Queue _responses = new(); - private readonly Action _captureAction; - - public MultiCapturingHttpMessageHandler(Action captureAction) - { - _captureAction = captureAction; - } - - public void QueueResponse(HttpResponseMessage resp) => _responses.Enqueue(resp); - - protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) - { - // Capture each request in sequence for integration test validation - _captureAction(request); - - if (_responses.Count == 0) - return Task.FromResult(new HttpResponseMessage(HttpStatusCode.NotFound) { Content = new StringContent("") }); - - var resp = _responses.Dequeue(); - return Task.FromResult(resp); - } -} diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/MosTokenServiceCacheTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/MosTokenServiceCacheTests.cs deleted file mode 100644 index 22d0b970..00000000 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/MosTokenServiceCacheTests.cs +++ /dev/null @@ -1,213 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -using System.Globalization; -using FluentAssertions; -using Microsoft.Agents.A365.DevTools.Cli.Helpers; -using Microsoft.Agents.A365.DevTools.Cli.Models; -using Microsoft.Agents.A365.DevTools.Cli.Services; -using Microsoft.Extensions.Logging; -using NSubstitute; - -namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Services; - -/// -/// Tests must run sequentially because they modify a shared cache file -/// at ~/.a365/mos-token-cache.json. -/// -[CollectionDefinition("MosTokenCacheTests", DisableParallelization = true)] -public class MosTokenCacheTestCollection { } - -[Collection("MosTokenCacheTests")] -public class MosTokenServiceCacheTests : IDisposable -{ - private readonly ILogger _mockLogger; - private readonly IConfigService _mockConfigService; - private readonly MosTokenService _service; - private readonly string _cacheFilePath; - private readonly string? _originalCacheContent; - - public MosTokenServiceCacheTests() - { - _mockLogger = Substitute.For>(); - _mockConfigService = Substitute.For(); - _service = new MosTokenService(_mockLogger, _mockConfigService); - - var cacheDir = FileHelper.GetSecureCrossOsDirectory(); - _cacheFilePath = Path.Combine(cacheDir, "mos-token-cache.json"); - - // Backup any existing cache file to restore after tests - _originalCacheContent = File.Exists(_cacheFilePath) - ? File.ReadAllText(_cacheFilePath) - : null; - } - - public void Dispose() - { - // Restore original cache state - if (_originalCacheContent != null) - { - File.WriteAllText(_cacheFilePath, _originalCacheContent); - } - else if (File.Exists(_cacheFilePath)) - { - File.Delete(_cacheFilePath); - } - } - - [Fact] - public async Task AcquireTokenAsync_CachedTokenWithFutureUtcExpiry_ReturnsCachedToken() - { - // Arrange - cache a token that expires 1 hour from now (UTC) - var futureUtc = DateTime.UtcNow.AddHours(1); - WriteCacheFile("prod", "cached-valid-token", futureUtc); - - // Act - var result = await _service.AcquireTokenAsync("prod"); - - // Assert - cached token returned without loading config - result.Should().Be("cached-valid-token"); - await _mockConfigService.DidNotReceive().LoadAsync(); - } - - [Fact] - public async Task AcquireTokenAsync_CachedTokenWithPastUtcExpiry_DoesNotReturnCachedToken() - { - // Arrange - cache a token that expired 1 hour ago (UTC) - var pastUtc = DateTime.UtcNow.AddHours(-1); - WriteCacheFile("prod", "expired-token", pastUtc); - SetupConfigForCacheMiss(); - - // Act - var result = await _service.AcquireTokenAsync("prod", cancellationToken: CancelledToken()); - - // Assert - expired token not returned, config was loaded (cache miss) - result.Should().NotBe("expired-token"); - await _mockConfigService.Received(1).LoadAsync(); - } - - [Fact] - public async Task AcquireTokenAsync_CachedTokenUtcZSuffix_ParsedAsUtcNotLocalTime() - { - // Regression test for #277. Without DateTimeStyles.AdjustToUniversal, - // DateTime.TryParse converts the "Z" suffix to local time. On IST (+5:30): - // Stored: "...12:00:00Z" Parsed: DateTime(17:30, Kind=Local) - // UtcNow(14:00) < 17:28 -> TRUE -> stale token returned (bug) - // Only catches the regression on UTC+ machines; see the Kind test below - // for the CI-reliable counterpart. - var expiredUtc = DateTime.UtcNow.AddHours(-3); - WriteCacheFile("prod", "stale-tz-token", expiredUtc); - SetupConfigForCacheMiss(); - - var result = await _service.AcquireTokenAsync("prod", cancellationToken: CancelledToken()); - - result.Should().NotBe("stale-tz-token"); - await _mockConfigService.Received(1).LoadAsync(); - } - - [Fact] - public void TryParseUtcTimestamp_WithAdjustToUniversal_ParsedAsUtcKindNotLocalTime() - { - // CI-reliable regression test for #277. On a UTC machine the buggy code - // produces Kind=Local with the same tick value as Kind=Utc, so comparison - // passes anyway — the service-level test above misses it. Checking Kind - // directly catches the regression on every machine including UTC CI runners. - const string utcZTimestamp = "2026-01-01T12:00:00.0000000Z"; - - var parsed = DateTime.TryParse( - utcZTimestamp, - CultureInfo.InvariantCulture, - DateTimeStyles.AdjustToUniversal, - out var result); - - parsed.Should().BeTrue(); - result.Kind.Should().Be(DateTimeKind.Utc); - result.Hour.Should().Be(12); - } - - [Fact] - public async Task AcquireTokenAsync_CachedTokenWithin2MinuteBuffer_DoesNotReturnCachedToken() - { - // Arrange - token expiring in 90 seconds (within 2-minute safety buffer) - var almostExpiredUtc = DateTime.UtcNow.AddSeconds(90); - WriteCacheFile("prod", "almost-expired-token", almostExpiredUtc); - SetupConfigForCacheMiss(); - - // Act - var result = await _service.AcquireTokenAsync("prod", cancellationToken: CancelledToken()); - - // Assert - token within buffer not returned - result.Should().NotBe("almost-expired-token"); - await _mockConfigService.Received(1).LoadAsync(); - } - - [Fact] - public async Task AcquireTokenAsync_CachedTokenForDifferentEnvironment_DoesNotReturnToken() - { - // Arrange - cache a valid token for "sdf" but request "prod" - var futureUtc = DateTime.UtcNow.AddHours(1); - WriteCacheFile("sdf", "sdf-only-token", futureUtc); - SetupConfigForCacheMiss(); - - // Act - var result = await _service.AcquireTokenAsync("prod", cancellationToken: CancelledToken()); - - // Assert - token for wrong environment not returned - result.Should().NotBe("sdf-only-token"); - await _mockConfigService.Received(1).LoadAsync(); - } - - [Fact] - public async Task AcquireTokenAsync_NoCacheFile_LoadsConfig() - { - // Arrange - ensure no cache file exists - if (File.Exists(_cacheFilePath)) - { - File.Delete(_cacheFilePath); - } - - SetupConfigForCacheMiss(); - - // Act - var result = await _service.AcquireTokenAsync("prod", cancellationToken: CancelledToken()); - - // Assert - no cache, falls through to config loading - await _mockConfigService.Received(1).LoadAsync(); - } - - /// - /// Write a MOS token cache JSON file with an ISO 8601 UTC expiry timestamp, - /// matching the format produced by MosTokenService.CacheToken(). - /// - private void WriteCacheFile(string environment, string token, DateTime expiryUtc) - { - var cacheDir = Path.GetDirectoryName(_cacheFilePath)!; - Directory.CreateDirectory(cacheDir); - - // Use the same format as CacheToken: expiry.ToUniversalTime().ToString("o") - // This produces "2026-02-18T17:00:00.0000000Z" with the "Z" suffix - var isoExpiry = expiryUtc.ToUniversalTime().ToString("o"); - var json = $$""" - { - "{{environment}}": { - "token": "{{token}}", - "expiry": "{{isoExpiry}}" - } - } - """; - File.WriteAllText(_cacheFilePath, json); - } - - private void SetupConfigForCacheMiss() - { - var config = new Agent365Config { TenantId = "test-tenant", ClientAppId = "test-client" }; - _mockConfigService.LoadAsync().Returns(Task.FromResult(config)); - } - - private static CancellationToken CancelledToken() - { - var cts = new CancellationTokenSource(); - cts.Cancel(); - return cts.Token; - } -} diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/MosTokenServiceTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/MosTokenServiceTests.cs deleted file mode 100644 index c4bf957c..00000000 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/MosTokenServiceTests.cs +++ /dev/null @@ -1,171 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -using FluentAssertions; -using Microsoft.Extensions.Logging; -using Microsoft.Agents.A365.DevTools.Cli.Services; -using Microsoft.Agents.A365.DevTools.Cli.Models; -using Microsoft.Agents.A365.DevTools.Cli.Constants; -using NSubstitute; - -namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Services; - -/// -/// Unit tests for MosTokenService. -/// -[Collection("MosTokenCacheTests")] -public class MosTokenServiceTests -{ - private readonly ILogger _mockLogger; - private readonly IConfigService _mockConfigService; - private readonly MosTokenService _service; - - public MosTokenServiceTests() - { - _mockLogger = Substitute.For>(); - _mockConfigService = Substitute.For(); - _service = new MosTokenService(_mockLogger, _mockConfigService); - } - - [Fact] - public async Task AcquireTokenAsync_WhenPersonalTokenProvided_ReturnsPersonalToken() - { - // Arrange - var personalToken = "test-personal-token"; - var environment = "prod"; - - // Act - var result = await _service.AcquireTokenAsync(environment, personalToken); - - // Assert - result.Should().Be(personalToken); - } - - [Fact] - public async Task AcquireTokenAsync_WhenConfigNotFound_ThrowsException() - { - // Arrange - var environment = "prod"; - _mockConfigService.LoadAsync().Returns(Task.FromException(new FileNotFoundException("Config not found"))); - - // Act - Func act = async () => await _service.AcquireTokenAsync(environment); - - // Assert - await act.Should().ThrowAsync(); - } - - [Fact] - public async Task AcquireTokenAsync_WhenTenantIdMissing_ReturnsNull() - { - // Arrange - var environment = "prod"; - Agent365Config? config = new Agent365Config { ClientAppId = "test-client-id", TenantId = "" }; - _mockConfigService.LoadAsync().Returns(Task.FromResult(config)); - - // Act - var result = await _service.AcquireTokenAsync(environment); - - // Assert - result.Should().BeNull(); - } - - [Fact] - public async Task AcquireTokenAsync_WhenUnsupportedEnvironment_ReturnsNull() - { - // Arrange - var environment = "invalid-env"; - Agent365Config? config = new Agent365Config { ClientAppId = "test-client-id", TenantId = "test-tenant-id" }; - _mockConfigService.LoadAsync().Returns(Task.FromResult(config)); - - // Act - var result = await _service.AcquireTokenAsync(environment); - - // Assert - result.Should().BeNull(); - } - - [Theory] - [InlineData("prod")] - [InlineData("sdf")] - [InlineData("test")] - [InlineData("gccm")] - [InlineData("gcch")] - [InlineData("dod")] - public async Task AcquireTokenAsync_SupportedEnvironments_LoadsConfig(string environment) - { - // Arrange - Agent365Config? config = new Agent365Config { ClientAppId = "test-client-id", TenantId = "test-tenant-id" }; - _mockConfigService.LoadAsync().Returns(Task.FromResult(config)); - var cts = new CancellationTokenSource(); - cts.Cancel(); // Cancel immediately to avoid browser prompt - - // Act & Assert - // Token acquisition will be cancelled before browser prompt - var result = await _service.AcquireTokenAsync(environment, cancellationToken: cts.Token); - - // Verify config was loaded (means we got past environment validation) - await _mockConfigService.Received(1).LoadAsync(); - - // Result will be null due to cancellation - result.Should().BeNull(); - } - - [Fact] - public void Constructor_WithValidParameters_CreatesInstance() - { - // Act - var service = new MosTokenService(_mockLogger, _mockConfigService); - - // Assert - service.Should().NotBeNull(); - } - - [Fact] - public async Task AcquireTokenAsync_NormalizesEnvironmentToLowercase() - { - // Arrange - Agent365Config? config = new Agent365Config { ClientAppId = "test-client-id", TenantId = "test-tenant-id" }; - _mockConfigService.LoadAsync().Returns(Task.FromResult(config)); - var cts = new CancellationTokenSource(); - cts.Cancel(); // Cancel immediately to avoid browser prompt - - // Act - var result = await _service.AcquireTokenAsync("PROD", cancellationToken: cts.Token); - - // Assert - await _mockConfigService.Received(1).LoadAsync(); - result.Should().BeNull(); - } - - [Fact] - public async Task AcquireTokenAsync_TrimsWhitespaceFromEnvironment() - { - // Arrange - Agent365Config? config = new Agent365Config { ClientAppId = "test-client-id", TenantId = "test-tenant-id" }; - _mockConfigService.LoadAsync().Returns(Task.FromResult(config)); - var cts = new CancellationTokenSource(); - cts.Cancel(); // Cancel immediately to avoid browser prompt - - // Act - var result = await _service.AcquireTokenAsync(" prod ", cancellationToken: cts.Token); - - // Assert - await _mockConfigService.Received(1).LoadAsync(); - result.Should().BeNull(); - } - - [Fact] - public async Task AcquireTokenAsync_WithPersonalToken_DoesNotLoadConfig() - { - // Arrange - var personalToken = "test-token"; - - // Act - var result = await _service.AcquireTokenAsync("prod", personalToken); - - // Assert - result.Should().Be(personalToken); - await _mockConfigService.DidNotReceive().LoadAsync(); - } -} From 106aa13abe376f1fb53833f697e5d03f7958df9a Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Wed, 11 Mar 2026 08:58:38 -0700 Subject: [PATCH 2/6] refactor: address code review issues in publish command cleanup - Remove unused agentBlueprintService parameter from CreateCommand signature and its callsite in Program.cs (CR-001) - Replace magic-number zip cap (4) and arbitrary file padding loop with a clean LINQ expression over known candidate filenames (CR-002) - Remove dead --verbose option binding in publish command handler; startup- level --verbose in Program.cs continues to work (CR-003) - Add CHANGELOG [Unreleased] entry for the breaking behavior change (CR-004) - Remove redundant Console.SetIn call inside zip creation test; constructor already sets it (CR-005) Co-Authored-By: Claude Sonnet 4.6 --- CHANGELOG.md | 3 ++ .../Commands/PublishCommand.cs | 28 ++++--------------- .../Program.cs | 2 +- .../Commands/PublishCommandTests.cs | 15 ---------- 4 files changed, 9 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e84f1fe2..08b3d990 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] +### Changed +- `a365 publish` no longer uploads the agent automatically. It now updates manifest IDs, creates `manifest.zip`, and prints instructions for manually uploading via Microsoft 365 Admin Center (Agents > All agents > Upload custom agent). + ### 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/PublishCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommand.cs index 9f2c93cb..1a2fe986 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommand.cs @@ -60,18 +60,13 @@ private static string GetProjectDirectory(Agent365Config config, ILogger logger) public static Command CreateCommand( ILogger logger, IConfigService configService, - AgentBlueprintService agentBlueprintService, ManifestTemplateService manifestTemplateService) { var command = new Command("publish", "Update manifest.json IDs and create a manifest package for upload to the Microsoft 365 Admin Center"); var dryRunOption = new Option("--dry-run", "Show changes without writing file or calling APIs"); - var verboseOption = new Option( - ["--verbose", "-v"], - description: "Enable verbose logging"); command.AddOption(dryRunOption); - command.AddOption(verboseOption); command.SetHandler(async (System.CommandLine.Invocation.InvocationContext context) => { @@ -217,25 +212,12 @@ public static Command CreateCommand( try { File.Delete(zipPath); } catch { /* ignore */ } } - // Identify files to include in zip; agenticUserTemplateManifest.json is explicitly listed - // to ensure it is always included regardless of other files present in the directory - var expectedFiles = new List(); + // Collect all known manifest files that exist; order is deterministic string[] candidateNames = ["manifest.json", "agenticUserTemplateManifest.json", "color.png", "outline.png", "logo.png", "icon.png"]; - foreach (var name in candidateNames) - { - var p = Path.Combine(manifestDir, name); - if (File.Exists(p)) expectedFiles.Add(p); - if (expectedFiles.Count == 4) break; - } - // If still fewer than 4, add any other files to reach 4 (non recursive) - if (expectedFiles.Count < 4) - { - foreach (var f in Directory.EnumerateFiles(manifestDir).Where(f => !expectedFiles.Contains(f))) - { - expectedFiles.Add(f); - if (expectedFiles.Count == 4) break; - } - } + var expectedFiles = candidateNames + .Select(name => Path.Combine(manifestDir, name)) + .Where(File.Exists) + .ToList(); if (expectedFiles.Count == 0) { diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs index fc78987a..ef379586 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs @@ -125,7 +125,7 @@ static async Task Main(string[] args) 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(PublishCommand.CreateCommand(publishLogger, configService, agentBlueprintService, manifestTemplateService)); + rootCommand.AddCommand(PublishCommand.CreateCommand(publishLogger, configService, manifestTemplateService)); // Wrap all command handlers with exception handling // Build with middleware for global exception handling 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 14561ed6..9e903508 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 @@ -28,7 +28,6 @@ public class PublishCommandTests : IDisposable { private readonly ILogger _logger; private readonly IConfigService _configService; - private readonly AgentBlueprintService _blueprintService; private readonly ManifestTemplateService _manifestTemplateService; private readonly TextReader _originalConsoleIn = Console.In; @@ -37,13 +36,6 @@ public PublishCommandTests() _logger = Substitute.For>(); _configService = Substitute.For(); - var graphApiService = Substitute.ForPartsOf(); - - // AgentBlueprintService needs (ILogger, GraphApiService) - _blueprintService = Substitute.ForPartsOf( - Substitute.For>(), - graphApiService); - // ManifestTemplateService needs only ILogger _manifestTemplateService = Substitute.ForPartsOf( Substitute.For>()); @@ -71,7 +63,6 @@ public async Task PublishCommand_WithMissingBlueprintId_ShouldReturnExitCode1() var command = PublishCommand.CreateCommand( _logger, _configService, - _blueprintService, _manifestTemplateService); var root = new RootCommand(); @@ -120,7 +111,6 @@ public async Task PublishCommand_WithDryRun_ShouldReturnExitCode0() var command = PublishCommand.CreateCommand( _logger, _configService, - _blueprintService, _manifestTemplateService); var root = new RootCommand(); @@ -173,13 +163,9 @@ public async Task PublishCommand_WithValidConfig_CreatesZipAndReturnsExitCode0() }; _configService.LoadAsync().Returns(config); - // Redirect stdin so interactive prompts auto-answer - Console.SetIn(new StringReader("n\n\n")); - var command = PublishCommand.CreateCommand( _logger, _configService, - _blueprintService, _manifestTemplateService); var root = new RootCommand(); @@ -211,7 +197,6 @@ public async Task PublishCommand_WithException_ShouldReturnExitCode1() var command = PublishCommand.CreateCommand( _logger, _configService, - _blueprintService, _manifestTemplateService); var root = new RootCommand(); From 5ee64c1f37f94882ad2254f0767ff4a4c22bffc4 Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Wed, 11 Mar 2026 09:46:20 -0700 Subject: [PATCH 3/6] fix: resolve post-merge build errors from main refactoring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Update Program.cs to use AzureAuthValidator directly (replaces removed IAzureValidator/AzureValidator), remove AzureWebAppCreator resolution, and pass authValidator to CleanupCommand - Delete MosPrerequisitesRequirementCheck and its test — this check wraps the deleted PublishHelpers and is no longer needed after the publish command was stripped to zip-only Co-Authored-By: Claude Sonnet 4.6 --- .../Program.cs | 16 +-- .../MosPrerequisitesRequirementCheck.cs | 72 ---------- .../MosPrerequisitesRequirementCheckTests.cs | 128 ------------------ 3 files changed, 5 insertions(+), 211 deletions(-) delete mode 100644 src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/MosPrerequisitesRequirementCheck.cs delete mode 100644 src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Requirements/MosPrerequisitesRequirementCheckTests.cs diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs index ef379586..3bfe0e49 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 azureAuthValidator = serviceProvider.GetRequiredService(); var toolingService = serviceProvider.GetRequiredService(); // Get services needed by commands @@ -101,7 +101,6 @@ static async Task Main(string[] args) var agentBlueprintService = serviceProvider.GetRequiredService(); var blueprintLookupService = serviceProvider.GetRequiredService(); var federatedCredentialService = serviceProvider.GetRequiredService(); - var webAppCreator = serviceProvider.GetRequiredService(); var platformDetector = serviceProvider.GetRequiredService(); var processService = serviceProvider.GetRequiredService(); var clientAppValidator = serviceProvider.GetRequiredService(); @@ -110,11 +109,11 @@ static async Task Main(string[] args) rootCommand.AddCommand(DevelopCommand.CreateCommand(developLogger, configService, executor, authService, graphApiService, agentBlueprintService, processService)); rootCommand.AddCommand(DevelopMcpCommand.CreateCommand(developLogger, toolingService)); rootCommand.AddCommand(SetupCommand.CreateCommand(setupLogger, configService, executor, - deploymentService, botConfigurator, azureValidator, webAppCreator, platformDetector, graphApiService, agentBlueprintService, blueprintLookupService, federatedCredentialService, clientAppValidator)); + deploymentService, botConfigurator, azureAuthValidator, 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, azureAuthValidator, graphApiService, agentBlueprintService)); // Register ConfigCommand var configLoggerFactory = serviceProvider.GetRequiredService(); @@ -124,7 +123,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, azureAuthValidator)); rootCommand.AddCommand(PublishCommand.CreateCommand(publishLogger, configService, manifestTemplateService)); // Wrap all command handlers with exception handling @@ -231,8 +230,6 @@ private static void ConfigureServices(IServiceCollection services, LogLevel mini services.AddSingleton(); services.AddSingleton(); - // Add unified Azure validator - services.AddSingleton(); // Add multi-platform deployment services services.AddSingleton(); @@ -251,9 +248,6 @@ private static void ConfigureServices(IServiceCollection services, LogLevel mini services.AddSingleton(); // For AgentApplication.Create permission services.AddSingleton(); // For publish command template extraction - // Register AzureWebAppCreator for SDK-based web app creation - services.AddSingleton(); - // Register ProcessService for cross-platform process launching services.AddSingleton(); 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 deleted file mode 100644 index 55f60104..00000000 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/MosPrerequisitesRequirementCheck.cs +++ /dev/null @@ -1,72 +0,0 @@ -// 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/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Requirements/MosPrerequisitesRequirementCheckTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Requirements/MosPrerequisitesRequirementCheckTests.cs deleted file mode 100644 index 5b04c300..00000000 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Requirements/MosPrerequisitesRequirementCheckTests.cs +++ /dev/null @@ -1,128 +0,0 @@ -// 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"); - } -} From dc4a802371a4bb20b5de5c69715a846e6eacb554 Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Wed, 11 Mar 2026 10:04:50 -0700 Subject: [PATCH 4/6] refactor: clean up publish command output to follow az cli conventions - Replace verbose section headers and wall-of-text guidance with a terse column-aligned field list (version, name.short, name.full, descriptions, developer.*, icons) - Remove duplicate success messages; single 'Manifest updated: {path}' line - Move per-file zip log entries to LogDebug - Remove mixed Console.WriteLine / logger calls; output consistent through logger - Add blank line after interactive prompt block before 'Package created:' - Fix column alignment: replace 'color.png / outline.png' key with 'icons' - Add name.short > 30 char warning - Remove unused logger parameters from UpdateManifestFileAsync and UpdateAgenticUserManifestTemplateFileAsync - Update tests: remove Console.SetIn workarounds that are no longer needed for early-exit paths; add WithDisplayNameExceeding30Chars_LogsWarning test Co-Authored-By: Claude Sonnet 4.6 --- CHANGELOG.md | 4 +- .../Commands/PublishCommand.cs | 182 +++++------------- .../Commands/PublishCommandTests.cs | 161 ++++++---------- 3 files changed, 108 insertions(+), 239 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 07547b71..8cf864e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,10 +9,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### 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 - ### Changed -- `a365 publish` no longer uploads the agent automatically. It now updates manifest IDs, creates `manifest.zip`, and prints instructions for manually uploading via Microsoft 365 Admin Center (Agents > All agents > Upload custom agent). +- `a365 publish` updates manifest IDs, creates `manifest.zip`, and prints concise upload instructions for Microsoft 365 Admin Center (Agents > All agents > Upload custom agent). Interactive prompts removed; command is now fully scriptable. ### Fixed - macOS/Linux: device code fallback when browser authentication is unavailable (#309) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommand.cs index 1a2fe986..ff5f5091 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommand.cs @@ -13,18 +13,14 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Commands; /// -/// Publish command – updates manifest.json ids based on the generated agent blueprint id +/// Publish command – updates manifest.json IDs based on the agent blueprint ID /// and packages the manifest files into a zip ready for manual upload. /// public class PublishCommand { /// /// Gets the project directory from config, with fallback to current directory. - /// Ensures absolute path resolution for portability. /// - /// Configuration containing deploymentProjectPath - /// Logger for warnings - /// Absolute path to project directory private static string GetProjectDirectory(Agent365Config config, ILogger logger) { var projectPath = config.DeploymentProjectPath; @@ -35,7 +31,6 @@ private static string GetProjectDirectory(Agent365Config config, ILogger logger) return Environment.CurrentDirectory; } - // Resolve to absolute path (handles both relative and absolute paths) try { var absolutePath = Path.IsPathRooted(projectPath) @@ -62,9 +57,9 @@ public static Command CreateCommand( IConfigService configService, ManifestTemplateService manifestTemplateService) { - var command = new Command("publish", "Update manifest.json IDs and create a manifest package for upload to the Microsoft 365 Admin Center"); + var command = new Command("publish", "Update manifest IDs and create a package for upload to Microsoft 365 Admin Center"); - var dryRunOption = new Option("--dry-run", "Show changes without writing file or calling APIs"); + var dryRunOption = new Option("--dry-run", "Show changes without writing files"); command.AddOption(dryRunOption); @@ -76,12 +71,9 @@ public static Command CreateCommand( try { - // Load configuration using ConfigService var config = await configService.LoadAsync(); - - // Extract required values from config - var agentBlueprintDisplayName = config.AgentBlueprintDisplayName; var blueprintId = config.AgentBlueprintId; + var displayName = config.AgentBlueprintDisplayName; if (string.IsNullOrWhiteSpace(blueprintId)) { @@ -89,172 +81,120 @@ public static Command CreateCommand( return; } - // Use deploymentProjectPath from config for portability var baseDir = GetProjectDirectory(config, logger); var manifestDir = Path.Combine(baseDir, "manifest"); var manifestPath = Path.Combine(manifestDir, "manifest.json"); - var agenticUserManifestTemplatePath = Path.Combine(manifestDir, "agenticUserTemplateManifest.json"); + var agenticUserManifestPath = Path.Combine(manifestDir, "agenticUserTemplateManifest.json"); - logger.LogDebug("Using project directory: {BaseDir}", baseDir); - logger.LogDebug("Using manifest directory: {ManifestDir}", manifestDir); - logger.LogDebug("Using blueprint ID: {BlueprintId}", blueprintId); + logger.LogDebug("Project directory: {BaseDir}", baseDir); + logger.LogDebug("Blueprint ID: {BlueprintId}", blueprintId); - // If manifest directory doesn't exist, extract templates from embedded resources if (!Directory.Exists(manifestDir)) { - logger.LogInformation("Manifest directory not found. Extracting templates from embedded resources..."); + logger.LogInformation("Extracting manifest templates..."); Directory.CreateDirectory(manifestDir); if (!manifestTemplateService.ExtractTemplates(manifestDir)) { - logger.LogError("Failed to extract manifest templates from embedded resources"); + logger.LogError("Failed to extract manifest templates from embedded resources."); return; } - - logger.LogInformation("Successfully extracted manifest templates to {ManifestDir}", manifestDir); - logger.LogInformation("Please customize the manifest files before publishing"); } - // Ensure agenticUserTemplateManifest.json exists in the manifest directory. - // It may be missing if the manifest directory was created by a previous partial run - // or an older CLI version that did not include this file. - if (!File.Exists(agenticUserManifestTemplatePath)) + if (!File.Exists(agenticUserManifestPath)) { - logger.LogInformation("agenticUserTemplateManifest.json not found. Extracting from embedded resources..."); if (!manifestTemplateService.EnsureTemplateFile(manifestDir, "agenticUserTemplateManifest.json")) { - logger.LogError("Failed to extract agenticUserTemplateManifest.json from embedded resources"); + logger.LogError("Failed to extract agenticUserTemplateManifest.json from embedded resources."); return; } } if (!File.Exists(manifestPath)) { - logger.LogError("Manifest file not found at {Path}", manifestPath); - logger.LogError("Expected location based on deploymentProjectPath: {ProjectPath}", baseDir); + logger.LogError("Manifest not found: {Path}", manifestPath); return; } - string updatedManifest = await UpdateManifestFileAsync(logger, agentBlueprintDisplayName, blueprintId, manifestPath); - - string updatedAgenticUserManifestTemplate = await UpdateAgenticUserManifestTemplateFileAsync(logger, agentBlueprintDisplayName, blueprintId, agenticUserManifestTemplatePath); + var updatedManifest = await UpdateManifestFileAsync(displayName, blueprintId, manifestPath); + var updatedAgenticUserManifest = await UpdateAgenticUserManifestTemplateFileAsync(blueprintId, agenticUserManifestPath); if (dryRun) { - logger.LogInformation("DRY RUN: Updated manifest (not saved):\n{Json}", updatedManifest); - logger.LogInformation("DRY RUN: Updated agentic user manifest template (not saved):\n{Json}", updatedAgenticUserManifestTemplate); - logger.LogInformation("DRY RUN: Skipping zipping"); + logger.LogInformation("DRY RUN: manifest.json (not saved):\n{Json}", updatedManifest); + logger.LogInformation("DRY RUN: agenticUserTemplateManifest.json (not saved):\n{Json}", updatedAgenticUserManifest); isNormalExit = true; return; } await File.WriteAllTextAsync(manifestPath, updatedManifest); - logger.LogInformation("Manifest updated successfully with agentBlueprintId {Id}", blueprintId); - - await File.WriteAllTextAsync(agenticUserManifestTemplatePath, updatedAgenticUserManifestTemplate); - logger.LogInformation("Agentic user manifest template updated successfully with agentBlueprintId {Id}", blueprintId); + await File.WriteAllTextAsync(agenticUserManifestPath, updatedAgenticUserManifest); - logger.LogDebug("Manifest files written to disk"); - - // Interactive pause for user customization - logger.LogInformation(""); - logger.LogInformation("=== MANIFEST UPDATED ==="); - Console.WriteLine($"Location: {manifestPath}"); - logger.LogInformation(""); - logger.LogInformation(""); - logger.LogInformation("=== CUSTOMIZE YOUR AGENT MANIFEST ==="); + logger.LogInformation("Manifest updated: {Path}", manifestPath); logger.LogInformation(""); - logger.LogInformation("Please customize these fields before publishing:"); - logger.LogInformation(""); - logger.LogInformation(" Version ('version')"); - logger.LogInformation(" - Increment for republishing (e.g., 1.0.0 to 1.0.1)"); - logger.LogInformation(" - REQUIRED: Must be higher than previously published version"); - logger.LogInformation(""); - logger.LogInformation(" Agent Name ('name.short' and 'name.full')"); - logger.LogInformation(" - Make it descriptive and user-friendly"); - logger.LogInformation(" - Currently: {Name}", agentBlueprintDisplayName); - logger.LogInformation(" - IMPORTANT: 'name.short' must be 30 characters or less"); - logger.LogInformation(""); - logger.LogInformation(" Descriptions ('description.short' and 'description.full')"); - logger.LogInformation(" - Short: 1-2 sentences"); - logger.LogInformation(" - Full: Detailed capabilities"); - logger.LogInformation(""); - logger.LogInformation(" Developer Info ('developer.name', 'developer.websiteUrl', 'developer.privacyUrl')"); - logger.LogInformation(" - Should reflect your organization details"); - logger.LogInformation(""); - logger.LogInformation(" Icons"); - logger.LogInformation(" - Replace 'color.png' and 'outline.png' with your custom branding"); + logger.LogInformation("Customize before packaging:"); + logger.LogInformation(" version - increment for republishing (e.g., 1.0.1), must be higher than previous"); + + if (!string.IsNullOrWhiteSpace(displayName) && displayName.Length > 30) + logger.LogWarning(" name.short - EXCEEDS 30 chars ({Length}), currently: \"{Name}\" -- shorten before packaging", displayName.Length, displayName); + else + logger.LogInformation(" name.short - 30 chars max, currently: \"{Name}\"", displayName); + + logger.LogInformation(" name.full - displayed in Microsoft 365"); + logger.LogInformation(" description.short - 1-2 sentences"); + logger.LogInformation(" description.full - detailed capabilities"); + logger.LogInformation(" developer.* - name, websiteUrl, privacyUrl"); + logger.LogInformation(" icons - replace color.png and outline.png with your branding"); logger.LogInformation(""); - // Ask if user wants to open the file now (skip when stdin is not a terminal) if (!Console.IsInputRedirected) { Console.Write("Open manifest in your default editor now? (Y/n): "); var openResponse = Console.ReadLine()?.Trim().ToLowerInvariant(); - if (openResponse != "n" && openResponse != "no") - { FileHelper.TryOpenFileInDefaultEditor(manifestPath, logger); - } Console.Write("Press Enter when you have finished editing the manifest to continue: "); Console.Out.Flush(); Console.ReadLine(); + Console.WriteLine(); } - logger.LogInformation("Creating manifest package..."); - logger.LogInformation(""); - - // Create manifest.zip including the required files var zipPath = Path.Combine(manifestDir, "manifest.zip"); if (File.Exists(zipPath)) { try { File.Delete(zipPath); } catch { /* ignore */ } } - // Collect all known manifest files that exist; order is deterministic string[] candidateNames = ["manifest.json", "agenticUserTemplateManifest.json", "color.png", "outline.png", "logo.png", "icon.png"]; - var expectedFiles = candidateNames + var filesToZip = candidateNames .Select(name => Path.Combine(manifestDir, name)) .Where(File.Exists) .ToList(); - if (expectedFiles.Count == 0) + if (filesToZip.Count == 0) { - logger.LogError("No manifest files found to zip in {Dir}", manifestDir); + logger.LogError("No manifest files found in {Dir}", manifestDir); return; } using (var zipStream = new FileStream(zipPath, FileMode.Create, FileAccess.ReadWrite)) using (var archive = new ZipArchive(zipStream, ZipArchiveMode.Create)) { - foreach (var file in expectedFiles) + foreach (var file in filesToZip) { - var entryName = Path.GetFileName(file); - var entry = archive.CreateEntry(entryName, CompressionLevel.Optimal); + logger.LogDebug("Adding {File} to manifest.zip", Path.GetFileName(file)); + var entry = archive.CreateEntry(Path.GetFileName(file), CompressionLevel.Optimal); await using var entryStream = entry.Open(); await using var src = File.OpenRead(file); await src.CopyToAsync(entryStream); - logger.LogInformation("Added {File} to manifest.zip", entryName); } } - logger.LogInformation("Created archive {ZipPath}", zipPath); - // Print manual upload instructions - logger.LogInformation(""); - logger.LogInformation("=== NEXT STEP: UPLOAD YOUR AGENT ==="); - logger.LogInformation(""); - logger.LogInformation("Your manifest package is ready at:"); - Console.WriteLine($" {zipPath}"); - logger.LogInformation(""); - logger.LogInformation("To publish your agent to Microsoft 365:"); - logger.LogInformation(" 1. Go to the Microsoft 365 Admin Center (https://admin.microsoft.com)"); - logger.LogInformation(" 2. Navigate to Agents > All agents"); - logger.LogInformation(" 3. Click 'Upload custom agent' and upload the manifest.zip file"); - logger.LogInformation(""); - logger.LogInformation("For detailed upload instructions, see:"); - logger.LogInformation(" https://learn.microsoft.com/en-us/copilot/microsoft-365/agent-essentials/agent-lifecycle/agent-upload-agents"); + logger.LogInformation("Package created: {ZipPath}", zipPath); logger.LogInformation(""); + logger.LogInformation("To publish: https://admin.microsoft.com > Agents > All agents > Upload custom agent"); + logger.LogInformation("For details: https://learn.microsoft.com/en-us/copilot/microsoft-365/agent-essentials/agent-lifecycle/agent-upload-agents"); isNormalExit = true; } @@ -274,67 +214,45 @@ public static Command CreateCommand( return command; } - private static async Task UpdateManifestFileAsync(ILogger logger, string? agentBlueprintDisplayName, string blueprintId, string manifestPath) + private static async Task UpdateManifestFileAsync(string? displayName, string blueprintId, string manifestPath) { - // Load manifest as mutable JsonNode var manifestText = await File.ReadAllTextAsync(manifestPath); var node = JsonNode.Parse(manifestText) ?? new JsonObject(); - // Update top-level id node["id"] = blueprintId; - // Update name.short and name.full if agentBlueprintDisplayName is available - if (!string.IsNullOrWhiteSpace(agentBlueprintDisplayName)) + if (!string.IsNullOrWhiteSpace(displayName)) { if (node["name"] is not JsonObject nameObj) { nameObj = new JsonObject(); node["name"] = nameObj; } - else - { - nameObj = (JsonObject)node["name"]!; - } - nameObj["short"] = agentBlueprintDisplayName; - nameObj["full"] = agentBlueprintDisplayName; - logger.LogInformation("Updated manifest name to: {Name}", agentBlueprintDisplayName); + nameObj["short"] = displayName; + nameObj["full"] = displayName; } - // bots[0].botId if (node["bots"] is JsonArray bots && bots.Count > 0 && bots[0] is JsonObject botObj) - { botObj["botId"] = blueprintId; - } - // webApplicationInfo.id + resource if (node["webApplicationInfo"] is JsonObject webInfo) { webInfo["id"] = blueprintId; webInfo["resource"] = $"api://{blueprintId}"; } - // copilotAgents.customEngineAgents[0].id if (node["copilotAgents"] is JsonObject ca && ca["customEngineAgents"] is JsonArray cea && cea.Count > 0 && cea[0] is JsonObject ceObj) - { ceObj["id"] = blueprintId; - } - var updated = node.ToJsonString(new JsonSerializerOptions { WriteIndented = true }); - return updated; + return node.ToJsonString(new JsonSerializerOptions { WriteIndented = true }); } - private static async Task UpdateAgenticUserManifestTemplateFileAsync(ILogger logger, string? agentBlueprintDisplayName, string blueprintId, string agenticUserManifestTemplateFilePath) + private static async Task UpdateAgenticUserManifestTemplateFileAsync(string blueprintId, string agenticUserManifestPath) { - // Load manifest as mutable JsonNode - var agenticUserManifestTemplateFileContents = await File.ReadAllTextAsync(agenticUserManifestTemplateFilePath); - var node = JsonNode.Parse(agenticUserManifestTemplateFileContents) ?? new JsonObject(); - - // Update top-level id + var contents = await File.ReadAllTextAsync(agenticUserManifestPath); + var node = JsonNode.Parse(contents) ?? new JsonObject(); node["agentIdentityBlueprintId"] = blueprintId; - - var updated = node.ToJsonString(new JsonSerializerOptions { WriteIndented = true }); - return updated; + return node.ToJsonString(new JsonSerializerOptions { WriteIndented = true }); } - } 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 fb6b7acc..304a5389 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 @@ -18,11 +18,6 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Commands; [CollectionDefinition("PublishCommandTests", DisableParallelization = true)] public class PublishCommandTestCollection { } -/// -/// Tests for PublishCommand exit code behavior. -/// Tests are limited to paths that exit before the interactive Console.ReadLine() prompts -/// in the publish flow. -/// [Collection("PublishCommandTests")] public class PublishCommandTests : IDisposable { @@ -35,14 +30,10 @@ public PublishCommandTests() { _logger = Substitute.For>(); _configService = Substitute.For(); - - // ManifestTemplateService needs only ILogger _manifestTemplateService = Substitute.ForPartsOf( Substitute.For>()); - // Auto-answer any Console.ReadLine prompts so tests do not block in environments - // where stdin is not redirected (e.g. Visual Studio Test Explorer). - // Answers: "n" = don't open editor, "" = press Enter to continue. + // Auto-answer interactive prompts: "n" = skip editor, "" = press Enter to continue. Console.SetIn(new StringReader("n\n\n")); } @@ -51,30 +42,20 @@ public PublishCommandTests() [Fact] public async Task PublishCommand_WithMissingBlueprintId_ShouldReturnExitCode1() { - // Arrange - Return config with missing blueprintId (this is an error path) var config = new Agent365Config { - AgentBlueprintId = null, // Missing blueprintId triggers error + AgentBlueprintId = null, TenantId = "test-tenant", AgentBlueprintDisplayName = "Test Agent" }; _configService.LoadAsync().Returns(config); - var command = PublishCommand.CreateCommand( - _logger, - _configService, - _manifestTemplateService); - var root = new RootCommand(); - root.AddCommand(command); + root.AddCommand(PublishCommand.CreateCommand(_logger, _configService, _manifestTemplateService)); - // Act var exitCode = await root.InvokeAsync("publish"); - // Assert exitCode.Should().Be(1, "missing blueprintId should return exit code 1"); - - // Verify error was logged _logger.Received().Log( LogLevel.Error, Arg.Any(), @@ -86,44 +67,29 @@ public async Task PublishCommand_WithMissingBlueprintId_ShouldReturnExitCode1() [Fact] public async Task PublishCommand_WithDryRun_ShouldReturnExitCode0() { - // Arrange - Set up config for successful dry-run var tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); var manifestDir = Path.Combine(tempDir, "manifest"); Directory.CreateDirectory(manifestDir); try { - // Create minimal manifest files for dry-run - var manifestPath = Path.Combine(manifestDir, "manifest.json"); - var agenticUserManifestPath = Path.Combine(manifestDir, "agenticUserTemplateManifest.json"); - await File.WriteAllTextAsync(manifestPath, "{\"id\":\"old-id\"}"); - await File.WriteAllTextAsync(agenticUserManifestPath, "{\"id\":\"old-id\"}"); + await File.WriteAllTextAsync(Path.Combine(manifestDir, "manifest.json"), "{\"id\":\"old-id\"}"); + await File.WriteAllTextAsync(Path.Combine(manifestDir, "agenticUserTemplateManifest.json"), "{\"id\":\"old-id\"}"); - var config = new Agent365Config + _configService.LoadAsync().Returns(new Agent365Config { AgentBlueprintId = "test-blueprint-id", AgentBlueprintDisplayName = "Test Agent", TenantId = "test-tenant", - ClientAppId = "test-client-app-id", DeploymentProjectPath = tempDir - }; - _configService.LoadAsync().Returns(config); - - var command = PublishCommand.CreateCommand( - _logger, - _configService, - _manifestTemplateService); + }); var root = new RootCommand(); - root.AddCommand(command); + root.AddCommand(PublishCommand.CreateCommand(_logger, _configService, _manifestTemplateService)); - // Act - Run with --dry-run option var exitCode = await root.InvokeAsync("publish --dry-run"); - // Assert - exitCode.Should().Be(0, "dry-run is a normal exit and should return exit code 0"); - - // Verify dry-run log message was written + exitCode.Should().Be(0, "dry-run should return exit code 0"); _logger.Received().Log( LogLevel.Information, Arg.Any(), @@ -133,83 +99,95 @@ public async Task PublishCommand_WithDryRun_ShouldReturnExitCode0() } finally { - if (Directory.Exists(tempDir)) - { - Directory.Delete(tempDir, true); - } + if (Directory.Exists(tempDir)) Directory.Delete(tempDir, true); } } [Fact] public async Task PublishCommand_WithValidConfig_CreatesZipAndReturnsExitCode0() { - // Arrange var tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); var manifestDir = Path.Combine(tempDir, "manifest"); Directory.CreateDirectory(manifestDir); try { - var manifestPath = Path.Combine(manifestDir, "manifest.json"); - var agenticUserManifestPath = Path.Combine(manifestDir, "agenticUserTemplateManifest.json"); - await File.WriteAllTextAsync(manifestPath, "{\"id\":\"old-id\"}"); - await File.WriteAllTextAsync(agenticUserManifestPath, "{\"agentIdentityBlueprintId\":\"old-id\"}"); + await File.WriteAllTextAsync(Path.Combine(manifestDir, "manifest.json"), "{\"id\":\"old-id\"}"); + await File.WriteAllTextAsync(Path.Combine(manifestDir, "agenticUserTemplateManifest.json"), "{\"agentIdentityBlueprintId\":\"old-id\"}"); - var config = new Agent365Config + _configService.LoadAsync().Returns(new Agent365Config { AgentBlueprintId = "test-blueprint-id", AgentBlueprintDisplayName = "Test Agent", TenantId = "test-tenant", DeploymentProjectPath = tempDir - }; - _configService.LoadAsync().Returns(config); - - var command = PublishCommand.CreateCommand( - _logger, - _configService, - _manifestTemplateService); + }); var root = new RootCommand(); - root.AddCommand(command); + root.AddCommand(PublishCommand.CreateCommand(_logger, _configService, _manifestTemplateService)); - // Act var exitCode = await root.InvokeAsync("publish"); - // Assert - exitCode.Should().Be(0, "successful zip creation should return exit code 0"); + exitCode.Should().Be(0, "successful publish should return exit code 0"); File.Exists(Path.Combine(manifestDir, "manifest.zip")).Should().BeTrue("manifest.zip should be created"); } finally { - if (Directory.Exists(tempDir)) + if (Directory.Exists(tempDir)) Directory.Delete(tempDir, true); + } + } + + [Fact] + public async Task PublishCommand_WithDisplayNameExceeding30Chars_LogsWarning() + { + var tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); + var manifestDir = Path.Combine(tempDir, "manifest"); + Directory.CreateDirectory(manifestDir); + + try + { + await File.WriteAllTextAsync(Path.Combine(manifestDir, "manifest.json"), "{\"id\":\"old-id\"}"); + await File.WriteAllTextAsync(Path.Combine(manifestDir, "agenticUserTemplateManifest.json"), "{\"agentIdentityBlueprintId\":\"old-id\"}"); + + _configService.LoadAsync().Returns(new Agent365Config { - Directory.Delete(tempDir, true); - } + AgentBlueprintId = "test-blueprint-id", + AgentBlueprintDisplayName = "This Display Name Is Way Too Long For Short", + TenantId = "test-tenant", + DeploymentProjectPath = tempDir + }); + + var root = new RootCommand(); + root.AddCommand(PublishCommand.CreateCommand(_logger, _configService, _manifestTemplateService)); + + var exitCode = await root.InvokeAsync("publish"); + + exitCode.Should().Be(0); + _logger.Received().Log( + LogLevel.Warning, + Arg.Any(), + Arg.Is(o => o.ToString()!.Contains("EXCEEDS 30 chars")), + Arg.Any(), + Arg.Any>()); + } + finally + { + if (Directory.Exists(tempDir)) Directory.Delete(tempDir, true); } } [Fact] public async Task PublishCommand_WithException_ShouldReturnExitCode1() { - // Arrange - Simulate exception during config loading _configService.LoadAsync() .Returns(_ => throw new InvalidOperationException("Test exception")); - var command = PublishCommand.CreateCommand( - _logger, - _configService, - _manifestTemplateService); - var root = new RootCommand(); - root.AddCommand(command); + root.AddCommand(PublishCommand.CreateCommand(_logger, _configService, _manifestTemplateService)); - // Act var exitCode = await root.InvokeAsync("publish"); - // Assert - exitCode.Should().Be(1, "exceptions should be caught and return exit code 1"); - - // Verify exception was logged + exitCode.Should().Be(1, "exceptions should return exit code 1"); _logger.Received().Log( LogLevel.Error, Arg.Any(), @@ -217,29 +195,4 @@ public async Task PublishCommand_WithException_ShouldReturnExitCode1() Arg.Is(ex => ex.Message == "Test exception"), Arg.Any>()); } - - /// - /// Documents the normal exit scenarios (exit code 0) and the main error scenarios (exit code 1). - /// - [Fact] - public void PublishCommand_DocumentsNormalExitScenarios() - { - var normalExitScenarios = new[] - { - "Dry-run: --dry-run specified, manifest updated but not saved", - "Complete success: manifest updated and manifest.zip created" - }; - - var errorExitScenarios = new[] - { - "Missing blueprintId in configuration", - "Failed to extract manifest templates", - "Manifest file not found", - "No manifest files found to zip", - "Exception thrown during execution" - }; - - normalExitScenarios.Should().HaveCount(2, "there are exactly 2 normal exit scenarios"); - errorExitScenarios.Length.Should().BeGreaterThan(3, "there are multiple error exit scenarios"); - } } From 465c16bf82da502712b901d532685216b50ed003 Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Wed, 11 Mar 2026 10:11:25 -0700 Subject: [PATCH 5/6] rename: MosConstants -> PowerPlatformConstants File and class name no longer reflect their contents after MOS upload infrastructure was removed. The remaining constants relate solely to the Power Platform API (CopilotStudio permissions), so rename to match. Co-Authored-By: Claude Sonnet 4.6 --- .../Commands/DevelopSubcommands/GetTokenSubcommand.cs | 2 +- .../Commands/SetupSubcommands/CopilotStudioSubcommand.cs | 6 +++--- .../Commands/SetupSubcommands/PermissionsSubcommand.cs | 4 ++-- .../{MosConstants.cs => PowerPlatformConstants.cs} | 2 +- .../Models/Agent365Config.cs | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) rename src/Microsoft.Agents.A365.DevTools.Cli/Constants/{MosConstants.cs => PowerPlatformConstants.cs} (94%) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopSubcommands/GetTokenSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopSubcommands/GetTokenSubcommand.cs index 4f8396c0..3c3ae52b 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopSubcommands/GetTokenSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopSubcommands/GetTokenSubcommand.cs @@ -392,7 +392,7 @@ private static (string resourceAppId, string displayName, string? url)? ResolveR return keyword.ToLowerInvariant() switch { "mcp" => (ConfigConstants.GetAgent365ToolsResourceAppId(environment), "Agent 365 Tools (MCP)", ConfigConstants.GetDiscoverEndpointUrl(environment)), - "powerplatform" => (MosConstants.PowerPlatformApiResourceAppId, "Power Platform API", null), + "powerplatform" => (PowerPlatformConstants.PowerPlatformApiResourceAppId, "Power Platform API", null), _ => null }; } 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 ffa5c27b..894b8bf1 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/CopilotStudioSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/CopilotStudioSubcommand.cs @@ -97,7 +97,7 @@ public static Command CreateCommand( logger.LogInformation("DRY RUN: Configure CopilotStudio Permissions"); logger.LogInformation("Would configure Power Platform API permissions:"); logger.LogInformation(" - Blueprint: {BlueprintId}", setupConfig.AgentBlueprintId); - logger.LogInformation(" - Resource: Power Platform API ({ResourceAppId})", MosConstants.PowerPlatformApiResourceAppId); + logger.LogInformation(" - Resource: Power Platform API ({ResourceAppId})", PowerPlatformConstants.PowerPlatformApiResourceAppId); logger.LogInformation(" - Scopes: CopilotStudio.Copilots.Invoke"); return; } @@ -144,9 +144,9 @@ await SetupHelpers.EnsureResourcePermissionsAsync( graphService, blueprintService, setupConfig, - MosConstants.PowerPlatformApiResourceAppId, + PowerPlatformConstants.PowerPlatformApiResourceAppId, "Power Platform API (CopilotStudio)", - new[] { MosConstants.PermissionNames.PowerPlatformCopilotStudioInvoke }, + new[] { PowerPlatformConstants.PermissionNames.PowerPlatformCopilotStudioInvoke }, logger, addToRequiredResourceAccess: false, setInheritablePermissions: true, 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 5a84c66b..a0292914 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs @@ -463,7 +463,7 @@ await SetupHelpers.EnsureResourcePermissionsAsync( graphService, blueprintService, setupConfig, - MosConstants.PowerPlatformApiResourceAppId, + PowerPlatformConstants.PowerPlatformApiResourceAppId, "Power Platform API", new[] { "Connectivity.Connections.Read" }, logger, @@ -514,7 +514,7 @@ private static async Task RemoveStaleCustomPermissionsAsync( ConfigConstants.GetAgent365ToolsResourceAppId(setupConfig.Environment), ConfigConstants.MessagingBotApiAppId, ConfigConstants.ObservabilityApiAppId, - MosConstants.PowerPlatformApiResourceAppId, + PowerPlatformConstants.PowerPlatformApiResourceAppId, AuthenticationConstants.MicrosoftGraphResourceAppId, }; diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Constants/MosConstants.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Constants/PowerPlatformConstants.cs similarity index 94% rename from src/Microsoft.Agents.A365.DevTools.Cli/Constants/MosConstants.cs rename to src/Microsoft.Agents.A365.DevTools.Cli/Constants/PowerPlatformConstants.cs index d57595ee..3b61dfb0 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Constants/MosConstants.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Constants/PowerPlatformConstants.cs @@ -6,7 +6,7 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Constants; /// /// Constants for Microsoft Power Platform API authentication and permissions /// -public static class MosConstants +public static class PowerPlatformConstants { /// /// Power Platform API resource app ID diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Models/Agent365Config.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Models/Agent365Config.cs index fff0578e..167fe8be 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Models/Agent365Config.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Models/Agent365Config.cs @@ -460,7 +460,7 @@ public bool IsBotInheritanceConfigured() var botResources = ResourceConsents .Where(rc => rc.ResourceAppId.Equals(ConfigConstants.MessagingBotApiAppId, StringComparison.OrdinalIgnoreCase) || rc.ResourceAppId.Equals(ConfigConstants.ObservabilityApiAppId, StringComparison.OrdinalIgnoreCase) || - rc.ResourceAppId.Equals(MosConstants.PowerPlatformApiResourceAppId, StringComparison.OrdinalIgnoreCase)) + rc.ResourceAppId.Equals(PowerPlatformConstants.PowerPlatformApiResourceAppId, StringComparison.OrdinalIgnoreCase)) .Where(rc => rc.InheritablePermissionsConfigured.HasValue) .ToList(); From 878dd64b15af527b8e547342a0888fff669783c7 Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Wed, 11 Mar 2026 10:30:45 -0700 Subject: [PATCH 6/6] fix: address PR #315 review comments - Delegate zip creation to ManifestTemplateService.CreateManifestZipAsync, removing duplicate inline implementation and aligning file list with the service's canonical set (fixes comments #1 and #2) - Move per-file zip log entries to LogDebug in ManifestTemplateService so the command output remains terse - Fix --dry-run option description: "without writing files or creating the zip" (fixes comment #3) - Handle null/empty displayName in name.short guidance with explicit warning instead of printing currently: "" (fixes comment #4) - Update test class summary to reflect current Console.In redirect approach (fixes comment #6) - Fix CHANGELOG: interactive prompts occur only in interactive terminals; stdin redirect suppresses them in scripts (fixes comment #7) - Comment #5 (PowerPlatformConstants summary) already resolved by prior rename Co-Authored-By: Claude Sonnet 4.6 --- CHANGELOG.md | 2 +- .../Commands/PublishCommand.cs | 34 ++++--------------- .../Services/ManifestTemplateService.cs | 4 +-- .../Commands/PublishCommandTests.cs | 3 +- 4 files changed, 11 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8cf864e3..3e711715 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - `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 ### Changed -- `a365 publish` updates manifest IDs, creates `manifest.zip`, and prints concise upload instructions for Microsoft 365 Admin Center (Agents > All agents > Upload custom agent). Interactive prompts removed; command is now fully scriptable. +- `a365 publish` updates manifest IDs, creates `manifest.zip`, and prints concise upload instructions for Microsoft 365 Admin Center (Agents > All agents > Upload custom agent). Interactive prompts only occur in interactive terminals; redirect stdin to suppress them in scripts. ### Fixed - macOS/Linux: device code fallback when browser authentication is unavailable (#309) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommand.cs index ff5f5091..97489321 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommand.cs @@ -6,7 +6,6 @@ using Microsoft.Agents.A365.DevTools.Cli.Services; using Microsoft.Extensions.Logging; using System.CommandLine; -using System.IO.Compression; using System.Text.Json; using System.Text.Json.Nodes; @@ -59,7 +58,7 @@ public static Command CreateCommand( { var command = new Command("publish", "Update manifest IDs and create a package for upload to Microsoft 365 Admin Center"); - var dryRunOption = new Option("--dry-run", "Show changes without writing files"); + var dryRunOption = new Option("--dry-run", "Show changes without writing files or creating the zip"); command.AddOption(dryRunOption); @@ -135,7 +134,9 @@ public static Command CreateCommand( logger.LogInformation("Customize before packaging:"); logger.LogInformation(" version - increment for republishing (e.g., 1.0.1), must be higher than previous"); - if (!string.IsNullOrWhiteSpace(displayName) && displayName.Length > 30) + if (string.IsNullOrWhiteSpace(displayName)) + logger.LogWarning(" name.short - not set; edit manifest.json to provide a short name (30 chars max) before packaging"); + else if (displayName.Length > 30) logger.LogWarning(" name.short - EXCEEDS 30 chars ({Length}), currently: \"{Name}\" -- shorten before packaging", displayName.Length, displayName); else logger.LogInformation(" name.short - 30 chars max, currently: \"{Name}\"", displayName); @@ -161,36 +162,13 @@ public static Command CreateCommand( } var zipPath = Path.Combine(manifestDir, "manifest.zip"); - if (File.Exists(zipPath)) - { - try { File.Delete(zipPath); } catch { /* ignore */ } - } - string[] candidateNames = ["manifest.json", "agenticUserTemplateManifest.json", "color.png", "outline.png", "logo.png", "icon.png"]; - var filesToZip = candidateNames - .Select(name => Path.Combine(manifestDir, name)) - .Where(File.Exists) - .ToList(); - - if (filesToZip.Count == 0) + if (!await manifestTemplateService.CreateManifestZipAsync(manifestDir, zipPath)) { - logger.LogError("No manifest files found in {Dir}", manifestDir); + logger.LogError("Failed to create manifest package in {Dir}", manifestDir); return; } - using (var zipStream = new FileStream(zipPath, FileMode.Create, FileAccess.ReadWrite)) - using (var archive = new ZipArchive(zipStream, ZipArchiveMode.Create)) - { - foreach (var file in filesToZip) - { - logger.LogDebug("Adding {File} to manifest.zip", Path.GetFileName(file)); - var entry = archive.CreateEntry(Path.GetFileName(file), CompressionLevel.Optimal); - await using var entryStream = entry.Open(); - await using var src = File.OpenRead(file); - await src.CopyToAsync(entryStream); - } - } - logger.LogInformation("Package created: {ZipPath}", zipPath); logger.LogInformation(""); logger.LogInformation("To publish: https://admin.microsoft.com > Agents > All agents > Upload custom agent"); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/ManifestTemplateService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/ManifestTemplateService.cs index 00d254d6..76202f94 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/ManifestTemplateService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/ManifestTemplateService.cs @@ -247,10 +247,10 @@ public async Task CreateManifestZipAsync(string workingDirectory, string o await using var fileStream = File.OpenRead(filePath); await fileStream.CopyToAsync(entryStream); - _logger.LogInformation("Added {File} to manifest.zip", fileName); + _logger.LogDebug("Added {File} to manifest.zip", fileName); } - _logger.LogInformation("Created manifest archive: {ZipPath}", outputZipPath); + _logger.LogDebug("Created manifest archive: {ZipPath}", outputZipPath); return true; } catch (Exception ex) 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 304a5389..5f6d3243 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 @@ -13,7 +13,8 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Commands; /// -/// Tests must run sequentially because some tests redirect Console.In (global state). +/// Tests must run sequentially because the constructor redirects Console.In (global state) +/// to auto-answer interactive prompts without blocking. /// [CollectionDefinition("PublishCommandTests", DisableParallelization = true)] public class PublishCommandTestCollection { }