From 5c2610d21266966f132471329203f549826e37b2 Mon Sep 17 00:00:00 2001 From: Deepali Garg Date: Sun, 3 May 2026 14:57:54 -0700 Subject: [PATCH 01/12] Making changes to the publish CLI to for MOS Publish, Custom connector creation and App registrations --- .../Commands/DevelopMcpCommand.cs | 186 ++----- .../Commands/PublishCommandExecutor.cs | 501 ++++++++++++++++++ .../Models/PublishMcpServerRequest.cs | 33 +- .../Models/PublishMcpServerResponse.cs | 42 +- .../DevelopMcpCommandRegressionTests.cs | 41 +- .../Commands/DevelopMcpCommandTests.cs | 24 +- 6 files changed, 630 insertions(+), 197 deletions(-) create mode 100644 src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs index f7a0d892..4f0d9f98 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs @@ -35,7 +35,7 @@ public static Command CreateCommand( // Add subcommands developMcpCommand.AddCommand(CreateListEnvironmentsSubcommand(logger, toolingService)); developMcpCommand.AddCommand(CreateListServersSubcommand(logger, toolingService)); - developMcpCommand.AddCommand(CreatePublishSubcommand(logger, toolingService)); + developMcpCommand.AddCommand(CreatePublishSubcommand(logger, toolingService, graphApiService)); developMcpCommand.AddCommand(CreateUnpublishSubcommand(logger, toolingService)); developMcpCommand.AddCommand(CreateApproveSubcommand(logger, toolingService)); developMcpCommand.AddCommand(CreateBlockSubcommand(logger, toolingService)); @@ -279,180 +279,62 @@ private static Command CreateListServersSubcommand( /// Creates the publish subcommand /// private static Command CreatePublishSubcommand( - ILogger logger, - IAgent365ToolingService toolingService) + ILogger logger, + IAgent365ToolingService toolingService, + GraphApiService? graphApiService) { - var command = new Command("publish", "Publish an MCP server to a Dataverse environment"); + var command = new Command("publish", "Publish an MCP server to a Dataverse environment. Creates the A365 Proxy + Public Clients Entra apps in your tenant, calls the platform publish endpoint, and back-fills redirect URIs and PPMI scope grants — same orchestration shape as register-external-mcp-server."); var envIdOption = new Option( ["--environment-id", "-e"], - description: "Dataverse environment ID" - ); - envIdOption.IsRequired = false; // Allow null so we can prompt + description: "Dataverse environment ID"); command.AddOption(envIdOption); var serverNameOption = new Option( ["--server-name", "-s"], - description: "MCP server name to publish" - ); - serverNameOption.IsRequired = false; // Allow null so we can prompt + description: "MCP server name to publish"); command.AddOption(serverNameOption); var aliasOption = new Option( ["--alias", "-a"], - description: "Alias for the MCP server" - ); + description: "Alias for the MCP server (used as the MCC row name and the CMS connector id)"); command.AddOption(aliasOption); var displayNameOption = new Option( ["--display-name", "-d"], - description: "Display name for the MCP server" - ); + description: "Display name for the MCP server (max 30 chars)"); command.AddOption(displayNameOption); - var dryRunOption = new Option( - name: "--dry-run", - description: "Show what would be done without executing" - ); - command.AddOption(dryRunOption); - - var verboseOption = new Option( - ["--verbose", "-v"], - description: "Enable verbose logging" - ); - command.AddOption(verboseOption); - - command.SetHandler(async (envId, serverName, alias, displayName, dryRun, verbose) => - { - _ = verbose; - try - { - // Validate and prompt for missing required arguments with security checks - if (string.IsNullOrWhiteSpace(envId)) - { - envId = InputValidator.PromptAndValidateRequiredInput("Enter Dataverse environment ID: ", "Environment ID"); - if (string.IsNullOrWhiteSpace(envId)) - { - logger.LogError("Environment ID is required"); - return; - } - } - else - { - // Validate provided environment ID - envId = InputValidator.ValidateInput(envId, "Environment ID"); - if (envId == null) - { - logger.LogError("Invalid environment ID format"); - return; - } - } - - if (string.IsNullOrWhiteSpace(serverName)) - { - serverName = InputValidator.PromptAndValidateRequiredInput("Enter MCP server name to publish: ", "Server name", 100); - if (string.IsNullOrWhiteSpace(serverName)) - { - logger.LogError("Server name is required"); - return; - } - } - else - { - // Validate provided server name - serverName = InputValidator.ValidateInput(serverName, "Server name"); - if (serverName == null) - { - logger.LogError("Invalid server name format"); - return; - } - } - - logger.LogInformation("Starting publish operation for server {ServerName} in environment {EnvId}...", serverName, envId); - - if (dryRun) - { - logger.LogInformation("[DRY RUN] Would read config from a365.config.json"); - logger.LogInformation("[DRY RUN] Would publish MCP server {ServerName} to environment {EnvId}", serverName, envId); - logger.LogInformation("[DRY RUN] Alias: {Alias}", alias ?? "[would prompt]"); - logger.LogInformation("[DRY RUN] Display Name: {DisplayName}", displayName ?? "[would prompt]"); - await Task.CompletedTask; - return; - } - - // Validate and prompt for missing optional values with security checks - if (string.IsNullOrWhiteSpace(alias)) - { - alias = InputValidator.PromptAndValidateRequiredInput("Enter alias for the MCP server: ", "Alias", 50); - if (string.IsNullOrWhiteSpace(alias)) - { - logger.LogError("Alias is required"); - return; - } - } - else - { - // Validate provided alias - alias = InputValidator.ValidateInput(alias, "Alias", maxLength: 50); - if (alias == null) - { - logger.LogError("Invalid alias format"); - return; - } - } - - if (string.IsNullOrWhiteSpace(displayName)) - { - displayName = InputValidator.PromptAndValidateRequiredInput("Enter display name for the MCP server: ", "Display name", 100); - if (string.IsNullOrWhiteSpace(displayName)) - { - logger.LogError("Display name is required"); - return; - } - } - else - { - // Validate provided display name - displayName = InputValidator.ValidateInput(displayName, "Display name", maxLength: 100); - if (displayName == null) - { - logger.LogError("Invalid display name format"); - return; - } - } - } - catch (ArgumentException ex) - { - logger.LogError("Input validation failed: {Message}", ex.Message); - return; - } + var tenantIdOption = new Option( + ["--tenant-id", "-t"], + description: "Entra tenant ID for Entra app creation (defaults to current az login tenant)"); + command.AddOption(tenantIdOption); - // Create request - var request = new PublishMcpServerRequest - { - Alias = alias, - DisplayName = displayName - }; + var serviceTreeIdOption = new Option( + "--service-tree-id", + description: "ServiceTree ID for Entra app registration (required in Microsoft corporate tenants)"); + command.AddOption(serviceTreeIdOption); - // Call service - var response = await toolingService.PublishServerAsync(envId, serverName, request); + var dryRunOption = new Option("--dry-run", "Show what would be done without executing"); + command.AddOption(dryRunOption); - if (response == null || !response.IsSuccess) - { - if (response?.Message != null) - { - logger.LogError("Failed to publish MCP server {ServerName} to environment {EnvId}: {ErrorMessage}", serverName, envId, response.Message); - } - else - { - logger.LogError("Failed to publish MCP server {ServerName} to environment {EnvId}: No response received", serverName, envId); - } - return; - } + // Verbose is handled globally in Program.cs (sets LogLevel.Debug); declared here so the parser accepts -v. + command.AddOption(new Option(["--verbose", "-v"], description: "Enable verbose logging")); - logger.LogInformation("Successfully published MCP server {ServerName} to environment {EnvId}", serverName, envId); + command.SetHandler(async (context) => + { + var args = new RawPublishArgs( + EnvironmentId: context.ParseResult.GetValueForOption(envIdOption), + ServerName: context.ParseResult.GetValueForOption(serverNameOption), + Alias: context.ParseResult.GetValueForOption(aliasOption), + DisplayName: context.ParseResult.GetValueForOption(displayNameOption), + TenantId: context.ParseResult.GetValueForOption(tenantIdOption), + ServiceTreeId: context.ParseResult.GetValueForOption(serviceTreeIdOption), + DryRun: context.ParseResult.GetValueForOption(dryRunOption)); - }, envIdOption, serverNameOption, aliasOption, displayNameOption, dryRunOption, verboseOption); + var executor = new PublishCommandExecutor(logger, toolingService, graphApiService); + await executor.ExecuteAsync(args, context.GetCancellationToken()); + }); return command; } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs new file mode 100644 index 00000000..ae7df4be --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs @@ -0,0 +1,501 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +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.Extensions.Logging; + +namespace Microsoft.Agents.A365.DevTools.Cli.Commands; + +/// +/// Raw CLI arguments passed to the develop-mcp publish command. +/// +internal record RawPublishArgs( + string? EnvironmentId, + string? ServerName, + string? Alias, + string? DisplayName, + string? TenantId, + string? ServiceTreeId, + bool DryRun); + +/// +/// Orchestrates first-party MCP server publish in one CLI command. The shape mirrors +/// for BYO register: the CLI creates the Entra apps it has +/// delegated authority to create (A365 Proxy + Public Clients in the user's own tenant), calls the +/// platform publish endpoint with those credentials, and back-fills the apps' redirect URIs and +/// PPMI scope grants after the platform creates the CMS connector and PPMI identity. +/// +internal class PublishCommandExecutor +{ + private readonly ILogger _logger; + private readonly IAgent365ToolingService _toolingService; + private readonly GraphApiService? _graphApiService; + private readonly RetryHelper _retryHelper; + + internal PublishCommandExecutor( + ILogger logger, + IAgent365ToolingService toolingService, + GraphApiService? graphApiService) + { + _logger = logger; + _toolingService = toolingService; + _graphApiService = graphApiService; + _retryHelper = new RetryHelper(logger, maxRetries: 5, baseDelaySeconds: 3); + } + + private sealed record ResolvedInput + { + public required string EnvironmentId { get; init; } + public required string ServerName { get; init; } + public required string Alias { get; init; } + public required string DisplayName { get; init; } + public required bool DryRun { get; init; } + public string? TenantId { get; init; } + public string? ServiceTreeId { get; init; } + } + + private sealed record EntraAppSet( + string A365AppClientId, + string A365AppSecret, + string A365AppObjectId, + string A365AppName, + string? PublicClientsClientId, + string? PublicClientsObjectId, + string PublicClientsAppName); + + internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct = default) + { + var input = ResolveInputs(args); + if (input is null) return; + + DisplayPublishSummary(input); + + if (input.DryRun) + { + _logger.LogInformation("[DRY RUN] Would create Entra apps '{A365}' and '{PublicClients}' in tenant", $"{input.Alias}-A365Proxy", $"{input.Alias}-PublicClients"); + _logger.LogInformation("[DRY RUN] Would call publish endpoint and back-fill redirect URI + PPMI scope on the created apps"); + return; + } + + Console.Write("Proceed with publish? (y/N): "); + var confirmation = Console.ReadLine()?.Trim().ToLowerInvariant(); + if (confirmation != "y" && confirmation != "yes") + { + Console.WriteLine("Publish cancelled."); + return; + } + + Console.WriteLine(); + ct.ThrowIfCancellationRequested(); + Console.WriteLine($"Publishing MCP server '{input.ServerName}' as '{input.Alias}' to environment {input.EnvironmentId}..."); + + var tenantId = await DetectTenantIdAsync(input.TenantId); + if (tenantId is null) return; + + if (_graphApiService is null) + { + _logger.LogError("Graph API service is not available. Cannot create Entra applications."); + return; + } + + var warnings = new List(); + var apps = await CreateEntraAppsAsync(input, tenantId, warnings); + if (apps is null) return; + + ct.ThrowIfCancellationRequested(); + + var request = new PublishMcpServerRequest + { + Alias = input.Alias, + DisplayName = input.DisplayName, + A365ProxyClientId = Guid.Parse(apps.A365AppClientId), + A365ProxyClientSecret = apps.A365AppSecret, + PublicClientsAppId = apps.PublicClientsClientId, + }; + + PublishMcpServerResponse? publishResponse; + try + { + publishResponse = await _toolingService.PublishServerAsync(input.EnvironmentId, input.ServerName, request, ct); + } + catch (Exception ex) + { + _logger.LogError("Failed to publish MCP server '{ServerName}': {Error}", input.ServerName, ex.Message); + _logger.LogDebug("Exception details: {Exception}", ex.ToString()); + _logger.LogWarning("Entra app registrations were NOT rolled back. Delete them manually in the Azure portal if needed."); + return; + } + + if (publishResponse is null || !publishResponse.IsSuccess) + { + var errorMsg = publishResponse?.Message ?? "No response received"; + _logger.LogError("Failed to publish MCP server {ServerName}: {Error}", input.ServerName, errorMsg); + _logger.LogWarning("Entra app registrations were NOT rolled back. Delete them manually in the Azure portal if needed."); + return; + } + + _logger.LogDebug("Successfully published MCP server {ServerName}", input.ServerName); + + await ConfigureEntraAppsAsync(input, apps, publishResponse, tenantId, warnings, ct); + + DisplayResults(input, warnings); + } + + private ResolvedInput? ResolveInputs(RawPublishArgs args) + { + try + { + var environmentId = args.EnvironmentId; + if (string.IsNullOrWhiteSpace(environmentId)) + { + environmentId = DevelopMcpCommand.InputValidator.PromptAndValidateRequiredInput("Enter Dataverse environment ID: ", "Environment ID"); + if (string.IsNullOrWhiteSpace(environmentId)) { _logger.LogError("Environment ID is required"); return null; } + } + else + { + environmentId = DevelopMcpCommand.InputValidator.ValidateInput(environmentId, "Environment ID"); + if (environmentId == null) { _logger.LogError("Invalid environment ID format"); return null; } + } + + var serverName = args.ServerName; + if (string.IsNullOrWhiteSpace(serverName)) + { + serverName = DevelopMcpCommand.InputValidator.PromptAndValidateRequiredInput("Enter MCP server name to publish: ", "Server name", 100); + if (string.IsNullOrWhiteSpace(serverName)) { _logger.LogError("Server name is required"); return null; } + } + else + { + serverName = DevelopMcpCommand.InputValidator.ValidateInput(serverName, "Server name"); + if (serverName == null) { _logger.LogError("Invalid server name format"); return null; } + } + + var alias = args.Alias; + if (string.IsNullOrWhiteSpace(alias)) + { + alias = DevelopMcpCommand.InputValidator.PromptAndValidateRequiredInput("Enter alias for the MCP server: ", "Alias", 50); + if (string.IsNullOrWhiteSpace(alias)) { _logger.LogError("Alias is required"); return null; } + } + else + { + alias = DevelopMcpCommand.InputValidator.ValidateInput(alias, "Alias", maxLength: 50); + if (alias == null) { _logger.LogError("Invalid alias format"); return null; } + } + + var displayName = args.DisplayName; + if (string.IsNullOrWhiteSpace(displayName)) + { + displayName = DevelopMcpCommand.InputValidator.PromptAndValidateRequiredInput("Enter display name for the MCP server: ", "Display name", 100); + if (string.IsNullOrWhiteSpace(displayName)) { _logger.LogError("Display name is required"); return null; } + } + else + { + displayName = DevelopMcpCommand.InputValidator.ValidateInput(displayName, "Display name", maxLength: 100); + if (displayName == null) { _logger.LogError("Invalid display name format"); return null; } + } + + return new ResolvedInput + { + EnvironmentId = environmentId, + ServerName = serverName, + Alias = alias, + DisplayName = displayName, + DryRun = args.DryRun, + TenantId = args.TenantId, + ServiceTreeId = args.ServiceTreeId, + }; + } + catch (ArgumentException ex) + { + _logger.LogError("Input validation failed: {Message}", ex.Message); + return null; + } + } + + private void DisplayPublishSummary(ResolvedInput input) + { + Console.WriteLine(); + var prevColor = Console.ForegroundColor; + Console.ForegroundColor = ConsoleColor.Cyan; + Console.WriteLine("Publish Summary"); + Console.WriteLine("==============="); + Console.ForegroundColor = prevColor; + DevelopMcpCommand.WriteLabel(" Environment: "); Console.WriteLine(input.EnvironmentId); + DevelopMcpCommand.WriteLabel(" Server Name: "); Console.WriteLine(input.ServerName); + DevelopMcpCommand.WriteLabel(" Alias: "); Console.WriteLine(input.Alias); + DevelopMcpCommand.WriteLabel(" Display Name: "); Console.WriteLine(input.DisplayName); + Console.WriteLine(); + } + + private async Task DetectTenantIdAsync(string? userTenantId) + { + var tenantId = userTenantId; + if (string.IsNullOrWhiteSpace(tenantId)) + { + tenantId = await TenantDetectionHelper.DetectTenantIdAsync(null, _logger); + } + + if (string.IsNullOrWhiteSpace(tenantId)) + { + _logger.LogError("Tenant ID could not be determined. Pass --tenant-id or run 'az login'."); + return null; + } + + return tenantId; + } + + private async Task CreateEntraAppsAsync(ResolvedInput input, string tenantId, List warnings) + { + var a365AppName = $"{input.ServerName}-A365Proxy"; + var publicClientsAppName = $"{input.ServerName}-PublicClients"; + + _logger.LogDebug("Creating Entra application for A365 Proxy..."); + var a365App = await _graphApiService!.CreateEntraAppAsync(tenantId, a365AppName, serviceTreeId: input.ServiceTreeId); + if (a365App == null) + { + _logger.LogError("Failed to create Entra application '{AppName}'. Ensure you have Application.ReadWrite.All permission in the target tenant. Run with -v for details.", a365AppName); + return null; + } + _logger.LogInformation("Created Entra app '{AppName}' (clientId: {ClientId})", a365AppName, a365App.Value.ClientId); + + var a365Secret = await _graphApiService.AddAppPasswordAsync(tenantId, a365App.Value.ObjectId); + if (string.IsNullOrWhiteSpace(a365Secret)) + { + _logger.LogError("Failed to create secret for '{AppName}'. Run with -v for details.", a365AppName); + return null; + } + + if (string.IsNullOrWhiteSpace(a365App.Value.ClientId)) + { + _logger.LogError("A365 Proxy Entra application was created but returned an empty client ID"); + return null; + } + + _logger.LogDebug("Created A365 Proxy app: {ClientId}", a365App.Value.ClientId); + + string? publicClientsClientId = null; + string? publicClientsObjectId = null; + + _logger.LogDebug("Creating Entra application for Public Clients..."); + var copilotApp = await _graphApiService.CreateEntraAppAsync(tenantId, publicClientsAppName, serviceTreeId: input.ServiceTreeId); + if (copilotApp != null) + { + publicClientsClientId = copilotApp.Value.ClientId; + publicClientsObjectId = copilotApp.Value.ObjectId; + _logger.LogInformation("Created Entra app '{AppName}' (clientId: {ClientId})", publicClientsAppName, publicClientsClientId); + + var copilotRedirectUri = $"ms-appx-web://Microsoft.AAD.BrokerPlugin/{publicClientsClientId}"; + var publicClientUris = new[] { copilotRedirectUri, "http://localhost:8080/callback", "https://vscode.dev/redirect", "http://localhost" }; + try + { + var success = await _retryHelper.ExecuteWithRetryAsync( + async ct => await _graphApiService.UpdateAppPublicClientRedirectUrisAsync(tenantId, publicClientsObjectId, publicClientUris, ct), + result => !result); + if (!success) + { + var msg = $"Failed to set redirect URIs on Public Clients app '{publicClientsAppName}' after retries."; + _logger.LogError(msg); + warnings.Add(msg); + } + else + { + _logger.LogDebug( + "Set {RedirectUriCount} redirect URIs on '{AppName}' ({ObjectId}): {RedirectUris}", + publicClientUris.Length, + publicClientsAppName, + publicClientsObjectId, + string.Join(", ", publicClientUris)); + } + } + catch (Exception ex) + { + var msg = $"Failed to set redirect URIs on Public Clients app: {ex.Message}"; + _logger.LogError(msg); + warnings.Add(msg); + } + } + else + { + var msg = "Failed to create Public Clients Entra app. Continuing without it."; + _logger.LogWarning(msg); + warnings.Add(msg); + } + + return new EntraAppSet( + A365AppClientId: a365App.Value.ClientId, + A365AppSecret: a365Secret, + A365AppObjectId: a365App.Value.ObjectId, + A365AppName: a365AppName, + PublicClientsClientId: publicClientsClientId, + PublicClientsObjectId: publicClientsObjectId, + PublicClientsAppName: publicClientsAppName); + } + + private async Task ConfigureEntraAppsAsync( + ResolvedInput input, + EntraAppSet apps, + PublishMcpServerResponse response, + string tenantId, + List warnings, + CancellationToken ct = default) + { + var tasks = new List(); + var concurrentWarnings = new System.Collections.Concurrent.ConcurrentBag(); + + var a365RedirectUri = response.A365ProxyRedirectUri; + + if (!string.IsNullOrWhiteSpace(a365RedirectUri)) + { + tasks.Add(UpdateA365RedirectUrisAsync(tenantId, apps, a365RedirectUri, concurrentWarnings, ct)); + } + else + { + var msg = "A365 Proxy redirect URI was not returned by the server. Redirect URI configuration skipped."; + _logger.LogWarning(msg); + concurrentWarnings.Add(msg); + } + + var ppmiAppClientId = response.PpmiAppClientId; + Guid? ppmiScopeId = null; + if (!string.IsNullOrWhiteSpace(ppmiAppClientId)) + { + _logger.LogDebug("PPMI app provisioned: {PpmiAppClientId}", ppmiAppClientId); + try + { + ppmiScopeId = await _retryHelper.ExecuteWithRetryAsync( + async retryCt => await _graphApiService!.GetOAuth2PermissionScopeIdAsync( + tenantId, ppmiAppClientId, "Tools.ListInvoke.All", retryCt), + result => !result.HasValue, + cancellationToken: ct); + } + catch (Exception ex) + { + var msg = $"Could not find 'Tools.ListInvoke.All' scope on PPMI app {ppmiAppClientId} after retries: {ex.Message}. API permissions not added."; + _logger.LogError(msg); + concurrentWarnings.Add(msg); + } + } + + if (ppmiScopeId.HasValue) + { + tasks.Add(AddPpmiPermissionAsync(tenantId, apps.A365AppObjectId, apps.A365AppName, ppmiAppClientId!, ppmiScopeId.Value, concurrentWarnings, ct)); + + if (apps.PublicClientsObjectId != null) + { + tasks.Add(AddPpmiPermissionAsync(tenantId, apps.PublicClientsObjectId, apps.PublicClientsAppName, ppmiAppClientId!, ppmiScopeId.Value, concurrentWarnings, ct)); + } + } + else if (!string.IsNullOrWhiteSpace(ppmiAppClientId) && ppmiScopeId == null) + { + var msg = $"Could not find 'Tools.ListInvoke.All' scope on PPMI app {ppmiAppClientId}. API permissions not added."; + _logger.LogError(msg); + concurrentWarnings.Add(msg); + } + + await Task.WhenAll(tasks); + + foreach (var w in concurrentWarnings) + warnings.Add(w); + } + + private async Task UpdateA365RedirectUrisAsync( + string tenantId, + EntraAppSet apps, + string a365RedirectUri, + System.Collections.Concurrent.ConcurrentBag concurrentWarnings, + CancellationToken ct = default) + { + try + { + var a365TcUri = DevelopMcpCommand.AddTcPrefix(a365RedirectUri); + var a365NonTcUri = DevelopMcpCommand.RemoveTcPrefix(a365RedirectUri); + var a365Uris = DevelopMcpCommand.BuildRedirectUriList(a365RedirectUri, a365TcUri, a365NonTcUri); + _logger.LogDebug("Updating redirect URIs on '{AppName}' ({ObjectId})", apps.A365AppName, apps.A365AppObjectId); + var success = await _retryHelper.ExecuteWithRetryAsync( + async retryCt => await _graphApiService!.UpdateAppRedirectUrisAsync(tenantId, apps.A365AppObjectId, a365Uris, retryCt), + result => !result, + cancellationToken: ct); + if (!success) + { + var msg = $"Failed to update redirect URIs on A365 Proxy app '{apps.A365AppName}' after retries."; + _logger.LogError(msg); + concurrentWarnings.Add(msg); + } + else + { + _logger.LogInformation("Updated redirect URIs on '{AppName}'", apps.A365AppName); + } + } + catch (Exception ex) + { + var msg = $"Failed to update redirect URIs on A365 Proxy app: {ex.Message}"; + _logger.LogError(msg); + concurrentWarnings.Add(msg); + } + } + + private async Task AddPpmiPermissionAsync( + string tenantId, + string appObjectId, + string appName, + string ppmiAppClientId, + Guid ppmiScopeId, + System.Collections.Concurrent.ConcurrentBag concurrentWarnings, + CancellationToken ct = default) + { + try + { + _logger.LogDebug("Adding PPMI 'Tools.ListInvoke.All' permission on '{AppName}' ({ObjectId})", appName, appObjectId); + var success = await _retryHelper.ExecuteWithRetryAsync( + async retryCt => await _graphApiService!.AddRequiredResourceAccessAsync( + tenantId, appObjectId, ppmiAppClientId, ppmiScopeId, retryCt), + result => !result, + cancellationToken: ct); + if (!success) + { + var msg = $"Failed to add PPMI permission on '{appName}' after retries."; + _logger.LogError(msg); + concurrentWarnings.Add(msg); + } + else + { + _logger.LogInformation("Added API permission on '{AppName}'", appName); + } + } + catch (Exception ex) + { + var msg = $"Failed to add PPMI permission on '{appName}': {ex.Message}"; + _logger.LogError(msg); + concurrentWarnings.Add(msg); + } + } + + private void DisplayResults(ResolvedInput input, List warnings) + { + Console.WriteLine(); + if (warnings.Count == 0) + { + var prevColor = Console.ForegroundColor; + Console.ForegroundColor = ConsoleColor.Green; + Console.WriteLine($"MCP server '{input.ServerName}' published as '{input.Alias}' to environment {input.EnvironmentId}."); + Console.ForegroundColor = prevColor; + } + else + { + var prevColor = Console.ForegroundColor; + Console.ForegroundColor = ConsoleColor.Yellow; + Console.WriteLine($"MCP server '{input.ServerName}' was published with {warnings.Count} warning(s):"); + Console.ForegroundColor = prevColor; + Console.WriteLine(); + foreach (var w in warnings) + { + _logger.LogWarning(" - {Warning}", w); + } + } + + Console.WriteLine(); + Console.WriteLine($"Please ask your tenant admin to approve MCP server '{input.ServerName}' in the Microsoft 365 Admin Center."); + } +} diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerRequest.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerRequest.cs index 966def0b..67af70da 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerRequest.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerRequest.cs @@ -1,23 +1,46 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +using System; using System.Text.Json.Serialization; namespace Microsoft.Agents.A365.DevTools.Cli.Models; /// -/// Request model for publishing an MCP server to a Dataverse environment +/// Request model for publishing an MCP server to a Dataverse environment. /// public class PublishMcpServerRequest { /// - /// Alias for the MCP server + /// Alias for the MCP server. /// [JsonPropertyName("alias")] public required string Alias { get; set; } /// - /// Display name for the MCP server + /// Display name for the MCP server. /// [JsonPropertyName("DisplayName")] public required string DisplayName { get; set; } + + /// + /// A365 Proxy Entra app client id created CLI-side at publish time. Paired with + /// . When provided, the platform creates an A365 Proxy CMS + /// connector keyed by server name so the published server is reachable through Power Platform / Copilot. + /// + [JsonPropertyName("a365ProxyClientId")] + public Guid? A365ProxyClientId { get; set; } + + /// + /// A365 Proxy Entra app client secret. Paired with . + /// + [JsonPropertyName("a365ProxyClientSecret")] + public string? A365ProxyClientSecret { get; set; } + + /// + /// Public Clients (VS Code / Copilot CLI) Entra app client id created CLI-side. Carried in the + /// request so the platform echoes it back in the response and the CLI can wire the PPMI + /// Tools.ListInvoke.All scope onto it after publish completes. + /// + [JsonPropertyName("publicClientsAppId")] + public string? PublicClientsAppId { get; set; } } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerResponse.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerResponse.cs index 1b874682..af5b0ebe 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerResponse.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerResponse.cs @@ -1,28 +1,58 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +using System; using System.Text.Json.Serialization; namespace Microsoft.Agents.A365.DevTools.Cli.Models; /// -/// Response model for MCP server publish operation +/// Response model for MCP server publish operation. /// public class PublishMcpServerResponse { /// - /// Status of the publish operation + /// Status of the publish operation. /// [JsonPropertyName("Status")] public string? Status { get; set; } /// - /// Message from the API response + /// Message from the API response. /// [JsonPropertyName("Message")] public string? Message { get; set; } /// - /// Whether the operation was successful + /// PPMI app client id for the published server. Used by the CLI to look up the + /// Tools.ListInvoke.All scope id and grant it to the A365 Proxy + Public Clients Entra + /// apps after publish completes. + /// + [JsonPropertyName("PpmiAppClientId")] + public string? PpmiAppClientId { get; set; } + + /// + /// CMS connector id created at publish time for the A365 Proxy connector, or null when the CLI + /// didn't pass Entra app credentials (older CLI flow) and no connector was created. + /// + [JsonPropertyName("A365ProxyConnectorId")] + public string? A365ProxyConnectorId { get; set; } + + /// + /// OAuth redirect URI for the A365 Proxy connector. The CLI writes this onto the just-created + /// A365 Proxy Entra app's redirect URI list (with tc / non-tc variants) so OAuth flows complete. + /// + [JsonPropertyName("A365ProxyRedirectUri")] + public string? A365ProxyRedirectUri { get; set; } + + /// + /// Public Clients Entra app client id, echoed back from the request so post-response + /// orchestration can grant the PPMI scope onto it. + /// + [JsonPropertyName("PublicClientsAppId")] + public string? PublicClientsAppId { get; set; } + + /// + /// Whether the operation was successful. /// [JsonIgnore] public bool IsSuccess => Status?.Equals("Success", StringComparison.OrdinalIgnoreCase) ?? false; diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandRegressionTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandRegressionTests.cs index 50a9c375..d1d74b94 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandRegressionTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandRegressionTests.cs @@ -94,46 +94,35 @@ public async Task AzureCliStyleParameters_AreAcceptedCorrectly(string command, p result.Should().Be(0, $"Azure CLI style command should be accepted: {string.Join(" ", fullCommand)}"); } - [Fact] - public async Task ServiceIntegration_PublishCommand_PassesCorrectParameters() + [Fact] + public async Task ServiceIntegration_PublishCommand_AcceptsAllNamedParameters() { - // Core functionality test: Ensures publish command integration works correctly - + // Verifies the publish CLI parses every documented flag without error. The publish flow now + // orchestrates Entra app creation + redirect-URI back-fill via GraphApiService (mirroring + // register-external-mcp-server), so end-to-end "params flow to PublishServerAsync" can't be + // exercised here without mocking Graph too — that path is covered by the + // regression test and by manual E2E testing. + // Arrange var testEnvId = "test-environment-123"; var testServerName = "msdyn_TestServer"; var testAlias = "test-alias"; var testDisplayName = "Test Server Display Name"; - var mockResponse = new PublishMcpServerResponse + // Act — dry-run short-circuits the Graph + platform calls so this stays a pure CLI parsing test. + var result = await _command.InvokeAsync(new[] { - Status = "Success", - Message = "Server published successfully" - }; - - _mockToolingService.PublishServerAsync(testEnvId, testServerName, Arg.Any()) - .Returns(mockResponse); - - // Act - var result = await _command.InvokeAsync(new[] - { - "publish", + "publish", "--environment-id", testEnvId, "--server-name", testServerName, "--alias", testAlias, - "--display-name", testDisplayName + "--display-name", testDisplayName, + "--dry-run", }); - // Assert + // Assert — successful parse + dispatch, no service calls. result.Should().Be(0); - - await _mockToolingService.Received(1).PublishServerAsync( - testEnvId, - testServerName, - Arg.Is(req => - req.Alias == testAlias && - req.DisplayName == testDisplayName) - ); + await _mockToolingService.DidNotReceive().PublishServerAsync(Arg.Any(), Arg.Any(), Arg.Any()); } [Fact] diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandTests.cs index d53e9161..b28f0979 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandTests.cs @@ -115,31 +115,39 @@ public void PublishSubcommand_HasCorrectOptionsWithAliases() var command = DevelopMcpCommand.CreateCommand(_mockLogger, _mockToolingService); var subcommand = command.Subcommands.First(sc => sc.Name == "publish"); - // Assert - subcommand.Description.Should().Be("Publish an MCP server to a Dataverse environment"); - + // Assert — description copy was extended in the BYO-parity work to call out the Entra app + + // back-fill orchestration the executor now performs, so match on the Azure-CLI-style verb prefix + // rather than the full string. + subcommand.Description.Should().StartWith("Publish an MCP server to a Dataverse environment"); + var options = subcommand.Options.ToList(); - - // Verify all expected options exist + + // Verify all expected options exist (including tenant-id + service-tree-id added for the Entra + // app creation step that mirrors register-external-mcp-server). var optionNames = options.Select(o => o.Name).ToList(); optionNames.Should().Contain("environment-id"); optionNames.Should().Contain("server-name"); optionNames.Should().Contain("alias"); optionNames.Should().Contain("display-name"); + optionNames.Should().Contain("tenant-id"); + optionNames.Should().Contain("service-tree-id"); optionNames.Should().Contain("dry-run"); // Verify critical aliases for Azure CLI compliance var envOption = options.FirstOrDefault(o => o.Name == "environment-id"); envOption!.Aliases.Should().Contain("-e"); - + var serverOption = options.FirstOrDefault(o => o.Name == "server-name"); serverOption!.Aliases.Should().Contain("-s"); - + var aliasOption = options.FirstOrDefault(o => o.Name == "alias"); aliasOption!.Aliases.Should().Contain("-a"); - + var displayNameOption = options.FirstOrDefault(o => o.Name == "display-name"); displayNameOption!.Aliases.Should().Contain("-d"); + + var tenantOption = options.FirstOrDefault(o => o.Name == "tenant-id"); + tenantOption!.Aliases.Should().Contain("-t"); } [Fact] From a7b5aead205604d3b68dd875d30cf15c74c1c843 Mon Sep 17 00:00:00 2001 From: Deepali Garg Date: Mon, 4 May 2026 13:34:13 -0700 Subject: [PATCH 02/12] Switch to v2 --- .../Commands/PublishCommandExecutor.cs | 5 +- .../Services/Agent365ToolingService.cs | 90 ++++++++++++++++++- .../Services/IAgent365ToolingService.cs | 17 ++++ 3 files changed, 110 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs index ae7df4be..9278d1ea 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs @@ -119,7 +119,10 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct = def PublishMcpServerResponse? publishResponse; try { - publishResponse = await _toolingService.PublishServerAsync(input.EnvironmentId, input.ServerName, request, ct); + // Hits the v2 publish endpoint, which performs the full elevation orchestration + // (lazy PPMI, MOS upload, A365 Proxy CMS connector creation). v1's PublishServerAsync + // is preserved on the platform for any callers relying on the original behavior. + publishResponse = await _toolingService.PublishServerV2Async(input.EnvironmentId, input.ServerName, request, ct); } catch (Exception ex) { diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Agent365ToolingService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Agent365ToolingService.cs index 374357e9..47f935a7 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Agent365ToolingService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Agent365ToolingService.cs @@ -251,7 +251,7 @@ private string BuildListMcpServersUrl(string environment, string environmentId) } /// - /// Builds URL for publishing an MCP server to a Dataverse environment + /// Builds URL for publishing an MCP server to a Dataverse environment (v1) /// /// Environment name /// Dataverse environment ID @@ -263,6 +263,20 @@ private string BuildPublishMcpServerUrl(string environment, string environmentId return $"{baseUrl}/agents/dataverse/environments/{environmentId}/mcpServers/{serverName}/publish"; } + /// + /// Builds URL for the v2 publish endpoint, which performs the full elevation orchestration + /// (lazy PPMI, MOS upload, A365 Proxy CMS connector creation). + /// + /// Environment name + /// Dataverse environment ID + /// MCP server name + /// Full URL for v2 publish MCP server endpoint + private string BuildPublishMcpServerV2Url(string environment, string environmentId, string serverName) + { + var baseUrl = BuildAgent365ToolsBaseUrl(environment); + return $"{baseUrl}/agents/v2/dataverse/environments/{environmentId}/mcpServers/{serverName}/publish"; + } + /// /// Builds URL for unpublishing an MCP server from a Dataverse environment /// @@ -581,6 +595,80 @@ private string BuildGetMCPServerUrl(string environment) } } + /// + public async Task PublishServerV2Async( + string environmentId, + string serverName, + PublishMcpServerRequest request, + CancellationToken cancellationToken = default) + { + if (string.IsNullOrWhiteSpace(environmentId)) + throw new ArgumentException("Environment ID cannot be null or empty", nameof(environmentId)); + if (string.IsNullOrWhiteSpace(serverName)) + throw new ArgumentException("Server name cannot be null or empty", nameof(serverName)); + if (request == null) + throw new ArgumentNullException(nameof(request)); + + try + { + var endpointUrl = BuildPublishMcpServerV2Url(_environment, environmentId, serverName); + var correlationId = Internal.HttpClientFactory.GenerateCorrelationId(); + + _logger.LogDebug("Publishing (v2) MCP server {ServerName} to environment {EnvId} (CorrelationId: {CorrelationId})", serverName, environmentId, correlationId); + _logger.LogDebug("Environment: {Env}", _environment); + _logger.LogDebug("Endpoint URL: {Url}", endpointUrl); + + var audience = ConfigConstants.GetAgent365ToolsResourceAppId(_environment); + _logger.LogDebug("Acquiring access token for audience: {Audience}", audience); + + var loginHint = await AzCliHelper.ResolveLoginHintAsync(); + var authToken = await _authService.GetAccessTokenAsync(audience, userId: loginHint, ct: cancellationToken); + if (string.IsNullOrWhiteSpace(authToken)) + { + _logger.LogError("Failed to acquire authentication token"); + return null; + } + + using var httpClient = Internal.HttpClientFactory.CreateAuthenticatedClient(authToken, correlationId: correlationId); + + var requestPayload = JsonSerializer.Serialize(request); + var jsonContent = new StringContent(requestPayload, System.Text.Encoding.UTF8, "application/json"); + + LogRequest("POST", endpointUrl, requestPayload); + + using var response = await httpClient.PostAsync(endpointUrl, jsonContent, cancellationToken); + + var (isSuccess, responseContent) = await ValidateResponseAsync(response, "publish (v2) MCP server", cancellationToken); + if (!isSuccess) + { + return null; + } + + if (string.IsNullOrWhiteSpace(responseContent)) + { + return new PublishMcpServerResponse + { + Status = "Success", + Message = $"Successfully published {serverName}", + }; + } + + var publishResponse = JsonDeserializationHelper.DeserializeWithDoubleSerialization( + responseContent, _logger); + + return publishResponse ?? new PublishMcpServerResponse + { + Status = "Success", + Message = $"Successfully published {serverName}", + }; + } + catch (Exception ex) + { + _logger.LogError(ex, "Failed to publish (v2) MCP server {ServerName} to environment {EnvId}", serverName, environmentId); + return null; + } + } + /// public async Task UnpublishServerAsync( string environmentId, diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/IAgent365ToolingService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/IAgent365ToolingService.cs index dd110f90..30103ab3 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/IAgent365ToolingService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/IAgent365ToolingService.cs @@ -46,6 +46,23 @@ public interface IAgent365ToolingService PublishMcpServerRequest request, CancellationToken cancellationToken = default); + /// + /// Publishes an MCP server via the v2 endpoint, which performs the full elevation orchestration + /// (lazy PPMI provision, MOS upload, A365 Proxy CMS connector creation when Entra creds are + /// supplied in the request). v1 remains for callers relying on + /// the original side-effect-free behavior. + /// + /// Dataverse environment ID + /// MCP server name to publish + /// Publish request with alias, display name, and optional Entra app credentials + /// Cancellation token + /// Response from the publish operation, including PPMI app id, A365 Proxy connector id, and redirect URI + Task PublishServerV2Async( + string environmentId, + string serverName, + PublishMcpServerRequest request, + CancellationToken cancellationToken = default); + /// /// Unpublishes an MCP server from a Dataverse environment /// From 1509108db334e862208db1aff0a2490ec9e20ede Mon Sep 17 00:00:00 2001 From: Deepali Garg Date: Thu, 7 May 2026 20:55:58 -0700 Subject: [PATCH 03/12] Updating app id changes --- .../Commands/PublishCommandExecutor.cs | 50 ++++++++++++------- .../Models/PublishMcpServerResponse.cs | 20 ++++++-- .../Services/Agent365ToolingService.cs | 2 +- 3 files changed, 47 insertions(+), 25 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs index 9278d1ea..7f725ecf 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs @@ -360,39 +360,51 @@ private async Task ConfigureEntraAppsAsync( concurrentWarnings.Add(msg); } - var ppmiAppClientId = response.PpmiAppClientId; - Guid? ppmiScopeId = null; - if (!string.IsNullOrWhiteSpace(ppmiAppClientId)) + // Grant required-resource-access on the just-created A365 Proxy + Public Clients Entra apps. + // The platform resolves the right resource per server type (Custom: managedidentityid; app-based + // / Dataverse MCP: 1p mappings; fallback: platform's own app id) and returns both the resource + // app id and the scope name. We look up the scope guid on the resource app, then add it as + // requiredResourceAccess on each of the two Entra apps we created. + var resourceAppId = response.McpServerAppId; + var resourceScopeName = response.McpServerScope; + Guid? resourceScopeId = null; + if (!string.IsNullOrWhiteSpace(resourceAppId) && !string.IsNullOrWhiteSpace(resourceScopeName)) { - _logger.LogDebug("PPMI app provisioned: {PpmiAppClientId}", ppmiAppClientId); + _logger.LogDebug("Resolving scope '{ScopeName}' on underlying server app {AppId}", resourceScopeName, resourceAppId); try { - ppmiScopeId = await _retryHelper.ExecuteWithRetryAsync( + resourceScopeId = await _retryHelper.ExecuteWithRetryAsync( async retryCt => await _graphApiService!.GetOAuth2PermissionScopeIdAsync( - tenantId, ppmiAppClientId, "Tools.ListInvoke.All", retryCt), + tenantId, resourceAppId, resourceScopeName, retryCt), result => !result.HasValue, cancellationToken: ct); } catch (Exception ex) { - var msg = $"Could not find 'Tools.ListInvoke.All' scope on PPMI app {ppmiAppClientId} after retries: {ex.Message}. API permissions not added."; + var msg = $"Could not find '{resourceScopeName}' scope on app {resourceAppId} after retries: {ex.Message}. API permissions not added."; _logger.LogError(msg); concurrentWarnings.Add(msg); } } + else + { + var msg = $"Underlying server app id or scope was not returned by publish (appId='{resourceAppId}', scope='{resourceScopeName}'). API permissions not added."; + _logger.LogWarning(msg); + concurrentWarnings.Add(msg); + } - if (ppmiScopeId.HasValue) + if (resourceScopeId.HasValue) { - tasks.Add(AddPpmiPermissionAsync(tenantId, apps.A365AppObjectId, apps.A365AppName, ppmiAppClientId!, ppmiScopeId.Value, concurrentWarnings, ct)); + tasks.Add(AddRequiredResourceAccessAsync(tenantId, apps.A365AppObjectId, apps.A365AppName, resourceAppId!, resourceScopeId.Value, concurrentWarnings, ct)); if (apps.PublicClientsObjectId != null) { - tasks.Add(AddPpmiPermissionAsync(tenantId, apps.PublicClientsObjectId, apps.PublicClientsAppName, ppmiAppClientId!, ppmiScopeId.Value, concurrentWarnings, ct)); + tasks.Add(AddRequiredResourceAccessAsync(tenantId, apps.PublicClientsObjectId, apps.PublicClientsAppName, resourceAppId!, resourceScopeId.Value, concurrentWarnings, ct)); } } - else if (!string.IsNullOrWhiteSpace(ppmiAppClientId) && ppmiScopeId == null) + else if (!string.IsNullOrWhiteSpace(resourceAppId) && !string.IsNullOrWhiteSpace(resourceScopeName)) { - var msg = $"Could not find 'Tools.ListInvoke.All' scope on PPMI app {ppmiAppClientId}. API permissions not added."; + var msg = $"Could not find '{resourceScopeName}' scope on app {resourceAppId}. API permissions not added."; _logger.LogError(msg); concurrentWarnings.Add(msg); } @@ -439,26 +451,26 @@ private async Task UpdateA365RedirectUrisAsync( } } - private async Task AddPpmiPermissionAsync( + private async Task AddRequiredResourceAccessAsync( string tenantId, string appObjectId, string appName, - string ppmiAppClientId, - Guid ppmiScopeId, + string resourceAppId, + Guid resourceScopeId, System.Collections.Concurrent.ConcurrentBag concurrentWarnings, CancellationToken ct = default) { try { - _logger.LogDebug("Adding PPMI 'Tools.ListInvoke.All' permission on '{AppName}' ({ObjectId})", appName, appObjectId); + _logger.LogDebug("Adding required-resource-access for resource {ResourceAppId} on '{AppName}' ({ObjectId})", resourceAppId, appName, appObjectId); var success = await _retryHelper.ExecuteWithRetryAsync( async retryCt => await _graphApiService!.AddRequiredResourceAccessAsync( - tenantId, appObjectId, ppmiAppClientId, ppmiScopeId, retryCt), + tenantId, appObjectId, resourceAppId, resourceScopeId, retryCt), result => !result, cancellationToken: ct); if (!success) { - var msg = $"Failed to add PPMI permission on '{appName}' after retries."; + var msg = $"Failed to add required-resource-access on '{appName}' after retries."; _logger.LogError(msg); concurrentWarnings.Add(msg); } @@ -469,7 +481,7 @@ private async Task AddPpmiPermissionAsync( } catch (Exception ex) { - var msg = $"Failed to add PPMI permission on '{appName}': {ex.Message}"; + var msg = $"Failed to add required-resource-access on '{appName}': {ex.Message}"; _logger.LogError(msg); concurrentWarnings.Add(msg); } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerResponse.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerResponse.cs index af5b0ebe..999bae6b 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerResponse.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerResponse.cs @@ -23,12 +23,22 @@ public class PublishMcpServerResponse public string? Message { get; set; } /// - /// PPMI app client id for the published server. Used by the CLI to look up the - /// Tools.ListInvoke.All scope id and grant it to the A365 Proxy + Public Clients Entra - /// apps after publish completes. + /// Resolved underlying MCP server's Entra app client id. The platform picks the right source per + /// server type — Custom servers from managedidentityid on the mcpserver row, app-based + /// / Dataverse MCP servers from 1p server-to-app mappings, fallback to the platform's own app id. + /// The CLI uses this together with to look up the scope id and grant + /// required-resource-access on the A365 Proxy + Public Clients Entra apps. /// - [JsonPropertyName("PpmiAppClientId")] - public string? PpmiAppClientId { get; set; } + [JsonPropertyName("McpServerAppId")] + public string? McpServerAppId { get; set; } + + /// + /// Resolved OAuth scope name on the underlying MCP server's app. Paired with ; + /// the CLI calls Graph's GetOAuth2PermissionScopeId on (McpServerAppId, McpServerScope) to get the + /// scope guid for required-resource-access grants. + /// + [JsonPropertyName("McpServerScope")] + public string? McpServerScope { get; set; } /// /// CMS connector id created at publish time for the A365 Proxy connector, or null when the CLI diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Agent365ToolingService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Agent365ToolingService.cs index 47f935a7..b02cc0f8 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Agent365ToolingService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Agent365ToolingService.cs @@ -274,7 +274,7 @@ private string BuildPublishMcpServerUrl(string environment, string environmentId private string BuildPublishMcpServerV2Url(string environment, string environmentId, string serverName) { var baseUrl = BuildAgent365ToolsBaseUrl(environment); - return $"{baseUrl}/agents/v2/dataverse/environments/{environmentId}/mcpServers/{serverName}/publish"; + return $"{baseUrl}/agents/dataverse/environments/{environmentId}/mcpServers/{serverName}/publish/v2"; } /// From d3dd0ca2c363035eba473958cd8f545c0c4d52ca Mon Sep 17 00:00:00 2001 From: Deepali Garg Date: Wed, 20 May 2026 14:12:07 -0700 Subject: [PATCH 04/12] Addressing PR Comments --- .../Commands/DevelopMcpCommand.cs | 22 +- .../Commands/PublishCommandExecutor.cs | 142 +++++------ .../Commands/RegisterCommandExecutor.cs | 125 ++-------- .../Services/EntraAppFactory.cs | 153 ++++++++++++ .../Commands/DevelopMcpCommandTests.cs | 18 +- .../PublishCommandExecutorDryRunTests.cs | 66 ++++++ .../Services/EntraAppFactoryTests.cs | 224 ++++++++++++++++++ 7 files changed, 534 insertions(+), 216 deletions(-) create mode 100644 src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppFactory.cs create mode 100644 src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorDryRunTests.cs create mode 100644 src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppFactoryTests.cs diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs index 4f0d9f98..acc6dac0 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs @@ -283,7 +283,7 @@ private static Command CreatePublishSubcommand( IAgent365ToolingService toolingService, GraphApiService? graphApiService) { - var command = new Command("publish", "Publish an MCP server to a Dataverse environment. Creates the A365 Proxy + Public Clients Entra apps in your tenant, calls the platform publish endpoint, and back-fills redirect URIs and PPMI scope grants — same orchestration shape as register-external-mcp-server."); + var command = new Command("publish", "Publish an MCP server to a Dataverse environment."); var envIdOption = new Option( ["--environment-id", "-e"], @@ -293,11 +293,12 @@ private static Command CreatePublishSubcommand( var serverNameOption = new Option( ["--server-name", "-s"], description: "MCP server name to publish"); + serverNameOption.IsRequired = false; command.AddOption(serverNameOption); var aliasOption = new Option( ["--alias", "-a"], - description: "Alias for the MCP server (used as the MCC row name and the CMS connector id)"); + description: "Alias for the MCP server"); command.AddOption(aliasOption); var displayNameOption = new Option( @@ -305,16 +306,6 @@ private static Command CreatePublishSubcommand( description: "Display name for the MCP server (max 30 chars)"); command.AddOption(displayNameOption); - var tenantIdOption = new Option( - ["--tenant-id", "-t"], - description: "Entra tenant ID for Entra app creation (defaults to current az login tenant)"); - command.AddOption(tenantIdOption); - - var serviceTreeIdOption = new Option( - "--service-tree-id", - description: "ServiceTree ID for Entra app registration (required in Microsoft corporate tenants)"); - command.AddOption(serviceTreeIdOption); - var dryRunOption = new Option("--dry-run", "Show what would be done without executing"); command.AddOption(dryRunOption); @@ -328,8 +319,6 @@ private static Command CreatePublishSubcommand( ServerName: context.ParseResult.GetValueForOption(serverNameOption), Alias: context.ParseResult.GetValueForOption(aliasOption), DisplayName: context.ParseResult.GetValueForOption(displayNameOption), - TenantId: context.ParseResult.GetValueForOption(tenantIdOption), - ServiceTreeId: context.ParseResult.GetValueForOption(serviceTreeIdOption), DryRun: context.ParseResult.GetValueForOption(dryRunOption)); var executor = new PublishCommandExecutor(logger, toolingService, graphApiService); @@ -726,9 +715,6 @@ private static Command CreateRegisterExternalMcpServerSubcommand( var remoteScopesOption = new Option("--remote-scopes", description: "Scopes for the remote MCP server (e.g., 'api://{appId-guid}/{scopeName}' such as 'api://00000000-0000-0000-0000-000000000000/access_as_user')"); command.AddOption(remoteScopesOption); - var tenantIdOption = new Option(["--tenant-id", "-t"], description: "Entra tenant ID for app registration (defaults to current az login tenant)"); - command.AddOption(tenantIdOption); - var serviceTreeIdOption = new Option("--service-tree-id", description: "ServiceTree ID for Entra app registration (required in Microsoft corporate tenants)"); command.AddOption(serviceTreeIdOption); @@ -763,7 +749,7 @@ private static Command CreateRegisterExternalMcpServerSubcommand( ToolsInput: context.ParseResult.GetValueForOption(toolsOption), InputFile: context.ParseResult.GetValueForOption(inputFileOption), RemoteScopes: context.ParseResult.GetValueForOption(remoteScopesOption), - TenantId: context.ParseResult.GetValueForOption(tenantIdOption), + TenantId: null, ServiceTreeId: context.ParseResult.GetValueForOption(serviceTreeIdOption), SecretLifetimeMonths: context.ParseResult.GetValueForOption(secretLifetimeMonthsOption), PublisherName: context.ParseResult.GetValueForOption(publisherOption), diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs index 7f725ecf..30caab87 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs @@ -17,8 +17,6 @@ internal record RawPublishArgs( string? ServerName, string? Alias, string? DisplayName, - string? TenantId, - string? ServiceTreeId, bool DryRun); /// @@ -53,11 +51,9 @@ private sealed record ResolvedInput public required string Alias { get; init; } public required string DisplayName { get; init; } public required bool DryRun { get; init; } - public string? TenantId { get; init; } - public string? ServiceTreeId { get; init; } } - private sealed record EntraAppSet( + internal sealed record EntraAppSet( string A365AppClientId, string A365AppSecret, string A365AppObjectId, @@ -75,7 +71,7 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct = def if (input.DryRun) { - _logger.LogInformation("[DRY RUN] Would create Entra apps '{A365}' and '{PublicClients}' in tenant", $"{input.Alias}-A365Proxy", $"{input.Alias}-PublicClients"); + _logger.LogInformation("[DRY RUN] Would create Entra apps '{A365}' and '{PublicClients}' in tenant", $"{input.ServerName}-A365Proxy", $"{input.ServerName}-PublicClients"); _logger.LogInformation("[DRY RUN] Would call publish endpoint and back-fill redirect URI + PPMI scope on the created apps"); return; } @@ -92,7 +88,7 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct = def ct.ThrowIfCancellationRequested(); Console.WriteLine($"Publishing MCP server '{input.ServerName}' as '{input.Alias}' to environment {input.EnvironmentId}..."); - var tenantId = await DetectTenantIdAsync(input.TenantId); + var tenantId = await DetectTenantIdAsync(); if (tenantId is null) return; if (_graphApiService is null) @@ -107,11 +103,18 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct = def ct.ThrowIfCancellationRequested(); + if (!Guid.TryParse(apps.A365AppClientId, out var a365ProxyClientId)) + { + _logger.LogError("A365 Proxy Entra app returned an invalid client ID '{ClientId}'. Expected a GUID. Cannot continue publish.", apps.A365AppClientId); + await RollbackEntraAppsAsync(apps, tenantId, ct); + return; + } + var request = new PublishMcpServerRequest { Alias = input.Alias, DisplayName = input.DisplayName, - A365ProxyClientId = Guid.Parse(apps.A365AppClientId), + A365ProxyClientId = a365ProxyClientId, A365ProxyClientSecret = apps.A365AppSecret, PublicClientsAppId = apps.PublicClientsClientId, }; @@ -128,7 +131,7 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct = def { _logger.LogError("Failed to publish MCP server '{ServerName}': {Error}", input.ServerName, ex.Message); _logger.LogDebug("Exception details: {Exception}", ex.ToString()); - _logger.LogWarning("Entra app registrations were NOT rolled back. Delete them manually in the Azure portal if needed."); + await RollbackEntraAppsAsync(apps, tenantId, ct); return; } @@ -136,7 +139,7 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct = def { var errorMsg = publishResponse?.Message ?? "No response received"; _logger.LogError("Failed to publish MCP server {ServerName}: {Error}", input.ServerName, errorMsg); - _logger.LogWarning("Entra app registrations were NOT rolled back. Delete them manually in the Azure portal if needed."); + await RollbackEntraAppsAsync(apps, tenantId, ct); return; } @@ -190,12 +193,12 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct = def var displayName = args.DisplayName; if (string.IsNullOrWhiteSpace(displayName)) { - displayName = DevelopMcpCommand.InputValidator.PromptAndValidateRequiredInput("Enter display name for the MCP server: ", "Display name", 100); + displayName = DevelopMcpCommand.InputValidator.PromptAndValidateRequiredInput("Enter display name for the MCP server: ", "Display name", 30); if (string.IsNullOrWhiteSpace(displayName)) { _logger.LogError("Display name is required"); return null; } } else { - displayName = DevelopMcpCommand.InputValidator.ValidateInput(displayName, "Display name", maxLength: 100); + displayName = DevelopMcpCommand.InputValidator.ValidateInput(displayName, "Display name", maxLength: 30); if (displayName == null) { _logger.LogError("Invalid display name format"); return null; } } @@ -206,8 +209,6 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct = def Alias = alias, DisplayName = displayName, DryRun = args.DryRun, - TenantId = args.TenantId, - ServiceTreeId = args.ServiceTreeId, }; } catch (ArgumentException ex) @@ -232,17 +233,13 @@ private void DisplayPublishSummary(ResolvedInput input) Console.WriteLine(); } - private async Task DetectTenantIdAsync(string? userTenantId) + private async Task DetectTenantIdAsync() { - var tenantId = userTenantId; - if (string.IsNullOrWhiteSpace(tenantId)) - { - tenantId = await TenantDetectionHelper.DetectTenantIdAsync(null, _logger); - } + var tenantId = await TenantDetectionHelper.DetectTenantIdAsync(null, _logger); if (string.IsNullOrWhiteSpace(tenantId)) { - _logger.LogError("Tenant ID could not be determined. Pass --tenant-id or run 'az login'."); + _logger.LogError("Tenant ID could not be determined. Run 'az login' and try again."); return null; } @@ -251,89 +248,70 @@ private void DisplayPublishSummary(ResolvedInput input) private async Task CreateEntraAppsAsync(ResolvedInput input, string tenantId, List warnings) { - var a365AppName = $"{input.ServerName}-A365Proxy"; - var publicClientsAppName = $"{input.ServerName}-PublicClients"; + var factory = new EntraAppFactory(_logger, _graphApiService!, _retryHelper); - _logger.LogDebug("Creating Entra application for A365 Proxy..."); - var a365App = await _graphApiService!.CreateEntraAppAsync(tenantId, a365AppName, serviceTreeId: input.ServiceTreeId); - if (a365App == null) - { - _logger.LogError("Failed to create Entra application '{AppName}'. Ensure you have Application.ReadWrite.All permission in the target tenant. Run with -v for details.", a365AppName); - return null; - } - _logger.LogInformation("Created Entra app '{AppName}' (clientId: {ClientId})", a365AppName, a365App.Value.ClientId); + var a365 = await factory.CreateProxyAppAsync( + input.ServerName, tenantId, suffix: "A365Proxy", roleDisplay: "A365 Proxy", serviceTreeId: null); + if (a365 == null) return null; - var a365Secret = await _graphApiService.AddAppPasswordAsync(tenantId, a365App.Value.ObjectId); - if (string.IsNullOrWhiteSpace(a365Secret)) - { - _logger.LogError("Failed to create secret for '{AppName}'. Run with -v for details.", a365AppName); - return null; - } + var publicClients = await factory.CreatePublicClientsAppAsync( + input.ServerName, tenantId, serviceTreeId: null, warnings); - if (string.IsNullOrWhiteSpace(a365App.Value.ClientId)) + return new EntraAppSet( + A365AppClientId: a365.ClientId, + A365AppSecret: a365.Secret, + A365AppObjectId: a365.ObjectId, + A365AppName: a365.AppName, + PublicClientsClientId: publicClients.ClientId, + PublicClientsObjectId: publicClients.ObjectId, + PublicClientsAppName: publicClients.AppName); + } + + // Best-effort compensating delete for the Entra apps created in CreateEntraAppsAsync, run when + // the platform publish call fails after app creation. Each delete is wrapped independently so + // a failure on the first app doesn't skip the second. Failures are logged with both clientId + // and objectId so the user can clean up manually. + internal async Task RollbackEntraAppsAsync(EntraAppSet apps, string tenantId, CancellationToken ct = default) + { + if (_graphApiService is null) { - _logger.LogError("A365 Proxy Entra application was created but returned an empty client ID"); - return null; + _logger.LogWarning("Graph API service is unavailable; cannot roll back Entra apps '{A365}' / '{PublicClients}'. Delete them manually in the Azure portal.", apps.A365AppName, apps.PublicClientsAppName); + return; } - _logger.LogDebug("Created A365 Proxy app: {ClientId}", a365App.Value.ClientId); + _logger.LogInformation("Rolling back Entra app registrations created for failed publish..."); - string? publicClientsClientId = null; - string? publicClientsObjectId = null; + await DeleteOneAsync(apps.A365AppObjectId, apps.A365AppClientId, apps.A365AppName, ct); - _logger.LogDebug("Creating Entra application for Public Clients..."); - var copilotApp = await _graphApiService.CreateEntraAppAsync(tenantId, publicClientsAppName, serviceTreeId: input.ServiceTreeId); - if (copilotApp != null) + if (!string.IsNullOrWhiteSpace(apps.PublicClientsObjectId)) { - publicClientsClientId = copilotApp.Value.ClientId; - publicClientsObjectId = copilotApp.Value.ObjectId; - _logger.LogInformation("Created Entra app '{AppName}' (clientId: {ClientId})", publicClientsAppName, publicClientsClientId); + await DeleteOneAsync(apps.PublicClientsObjectId, apps.PublicClientsClientId, apps.PublicClientsAppName, ct); + } - var copilotRedirectUri = $"ms-appx-web://Microsoft.AAD.BrokerPlugin/{publicClientsClientId}"; - var publicClientUris = new[] { copilotRedirectUri, "http://localhost:8080/callback", "https://vscode.dev/redirect", "http://localhost" }; + async Task DeleteOneAsync(string objectId, string? clientId, string appName, CancellationToken cancellationToken) + { try { - var success = await _retryHelper.ExecuteWithRetryAsync( - async ct => await _graphApiService.UpdateAppPublicClientRedirectUrisAsync(tenantId, publicClientsObjectId, publicClientUris, ct), - result => !result); - if (!success) + var deleted = await _graphApiService!.DeleteEntraAppAsync(tenantId, objectId, cancellationToken); + if (deleted) { - var msg = $"Failed to set redirect URIs on Public Clients app '{publicClientsAppName}' after retries."; - _logger.LogError(msg); - warnings.Add(msg); + _logger.LogInformation("Rolled back Entra app '{AppName}' (objectId {ObjectId})", appName, objectId); } else { - _logger.LogDebug( - "Set {RedirectUriCount} redirect URIs on '{AppName}' ({ObjectId}): {RedirectUris}", - publicClientUris.Length, - publicClientsAppName, - publicClientsObjectId, - string.Join(", ", publicClientUris)); + _logger.LogError( + "Failed to roll back Entra app '{AppName}' (clientId {ClientId}, objectId {ObjectId}). Delete it manually in the Azure portal.", + appName, clientId ?? "", objectId); } } catch (Exception ex) { - var msg = $"Failed to set redirect URIs on Public Clients app: {ex.Message}"; - _logger.LogError(msg); - warnings.Add(msg); + _logger.LogError( + ex, + "Exception rolling back Entra app '{AppName}' (clientId {ClientId}, objectId {ObjectId}). Delete it manually in the Azure portal.", + appName, clientId ?? "", objectId); } } - else - { - var msg = "Failed to create Public Clients Entra app. Continuing without it."; - _logger.LogWarning(msg); - warnings.Add(msg); - } - - return new EntraAppSet( - A365AppClientId: a365App.Value.ClientId, - A365AppSecret: a365Secret, - A365AppObjectId: a365App.Value.ObjectId, - A365AppName: a365AppName, - PublicClientsClientId: publicClientsClientId, - PublicClientsObjectId: publicClientsObjectId, - PublicClientsAppName: publicClientsAppName); } private async Task ConfigureEntraAppsAsync( diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs index 20f1c01b..51a2c33c 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs @@ -582,127 +582,44 @@ private void DisplayRegistrationSummary(ResolvedInput input) private async Task CreateEntraAppsAsync( ResolvedInput input, string tenantId, List warnings) { - var a365AppName = $"{input.ServerName}-A365Proxy"; - var remoteProxyAppName = $"{input.ServerName}-RemoteProxy"; - var publicClientsAppName = $"{input.ServerName}-PublicClients"; - - _logger.LogDebug("Creating Entra application for A365 Proxy..."); - var a365App = await _graphApiService!.CreateEntraAppAsync(tenantId, a365AppName, serviceTreeId: input.ServiceTreeId); - if (a365App == null) - { - _logger.LogError("Failed to create Entra application '{AppName}'. Ensure you have Application.ReadWrite.All permission in the target tenant. Run with -v for details.", a365AppName); - return null; - } - _logger.LogInformation("Created Entra app '{AppName}' (clientId: {ClientId})", a365AppName, a365App.Value.ClientId); - - var a365Secret = await _graphApiService.AddAppPasswordAsync(tenantId, a365App.Value.ObjectId, lifetimeMonths: input.SecretLifetimeMonths); - if (string.IsNullOrWhiteSpace(a365Secret)) - { - _logger.LogError("Failed to create secret for '{AppName}'. Run with -v for details.", a365AppName); - return null; - } + var factory = new EntraAppFactory(_logger, _graphApiService!, _retryHelper); - if (string.IsNullOrWhiteSpace(a365App.Value.ClientId)) - { - _logger.LogError("A365 Proxy Entra application was created but returned an empty client ID"); - return null; - } - - _logger.LogDebug("Created A365 Proxy app: {ClientId}", a365App.Value.ClientId); + var a365 = await factory.CreateProxyAppAsync( + input.ServerName, tenantId, suffix: "A365Proxy", roleDisplay: "A365 Proxy", serviceTreeId: input.ServiceTreeId); + if (a365 == null) return null; string? remoteProxyClientId = null; string? remoteProxySecret = null; string? remoteProxyObjectId = null; + var remoteProxyAppName = $"{input.ServerName}-RemoteProxy"; if (input.IsEntra) { - _logger.LogDebug("Creating Entra application for Remote Proxy..."); - var remoteApp = await _graphApiService.CreateEntraAppAsync(tenantId, remoteProxyAppName, serviceTreeId: input.ServiceTreeId); - if (remoteApp == null) - { - _logger.LogError("Failed to create Entra application '{AppName}'. Ensure you have Application.ReadWrite.All permission in the target tenant. Run with -v for details.", remoteProxyAppName); - return null; - } - _logger.LogInformation("Created Entra app '{AppName}' (clientId: {ClientId})", remoteProxyAppName, remoteApp.Value.ClientId); - - remoteProxySecret = await _graphApiService.AddAppPasswordAsync(tenantId, remoteApp.Value.ObjectId, lifetimeMonths: input.SecretLifetimeMonths); - if (string.IsNullOrWhiteSpace(remoteProxySecret)) - { - _logger.LogError("Failed to create secret for '{AppName}'. Run with -v for details.", remoteProxyAppName); - return null; - } - - if (string.IsNullOrWhiteSpace(remoteApp.Value.ClientId)) - { - _logger.LogError("Remote Proxy Entra application was created but returned an empty client ID"); - return null; - } + var remote = await factory.CreateProxyAppAsync( + input.ServerName, tenantId, suffix: "RemoteProxy", roleDisplay: "Remote Proxy", serviceTreeId: input.ServiceTreeId); + if (remote == null) return null; - _logger.LogDebug("Created Remote Proxy app: {ClientId}", remoteApp.Value.ClientId); - remoteProxyClientId = remoteApp.Value.ClientId; - remoteProxyObjectId = remoteApp.Value.ObjectId; + remoteProxyClientId = remote.ClientId; + remoteProxySecret = remote.Secret; + remoteProxyObjectId = remote.ObjectId; + remoteProxyAppName = remote.AppName; } - string? publicClientsClientId = null; - string? publicClientsObjectId = null; - - _logger.LogDebug("Creating Entra application for Public Clients..."); - var copilotApp = await _graphApiService.CreateEntraAppAsync(tenantId, publicClientsAppName, serviceTreeId: input.ServiceTreeId); - if (copilotApp != null) - { - publicClientsClientId = copilotApp.Value.ClientId; - publicClientsObjectId = copilotApp.Value.ObjectId; - _logger.LogInformation("Created Entra app '{AppName}' (clientId: {ClientId})", publicClientsAppName, publicClientsClientId); - - var copilotRedirectUri = $"ms-appx-web://Microsoft.AAD.BrokerPlugin/{publicClientsClientId}"; - var publicClientUris = new[] { copilotRedirectUri, "http://localhost:8080/callback", "https://vscode.dev/redirect", "http://localhost" }; - try - { - var success = await _retryHelper.ExecuteWithRetryAsync( - async ct => await _graphApiService.UpdateAppPublicClientRedirectUrisAsync(tenantId, publicClientsObjectId, publicClientUris, ct), - result => !result); - if (!success) - { - var msg = $"Failed to set redirect URIs on Public Clients app '{publicClientsAppName}' after retries."; - _logger.LogError(msg); - warnings.Add(msg); - } - else - { - _logger.LogDebug( - "Set {RedirectUriCount} redirect URIs on '{AppName}' ({ObjectId}): {RedirectUris}", - publicClientUris.Length, - publicClientsAppName, - publicClientsObjectId, - string.Join(", ", publicClientUris)); - } - } - catch (Exception ex) - { - var msg = $"Failed to set redirect URIs on Public Clients app: {ex.Message}"; - _logger.LogError(msg); - warnings.Add(msg); - } - } - else - { - var msg = "Failed to create Public Clients Entra app. Continuing without it."; - _logger.LogWarning(msg); - warnings.Add(msg); - } + var publicClients = await factory.CreatePublicClientsAppAsync( + input.ServerName, tenantId, serviceTreeId: input.ServiceTreeId, warnings); return new EntraAppSet( - A365AppClientId: a365App.Value.ClientId, - A365AppSecret: a365Secret, - A365AppObjectId: a365App.Value.ObjectId, - A365AppName: a365AppName, + A365AppClientId: a365.ClientId, + A365AppSecret: a365.Secret, + A365AppObjectId: a365.ObjectId, + A365AppName: a365.AppName, RemoteProxyClientId: remoteProxyClientId, RemoteProxySecret: remoteProxySecret, RemoteProxyObjectId: remoteProxyObjectId, RemoteProxyAppName: remoteProxyAppName, - PublicClientsClientId: publicClientsClientId, - PublicClientsObjectId: publicClientsObjectId, - PublicClientsAppName: publicClientsAppName); + PublicClientsClientId: publicClients.ClientId, + PublicClientsObjectId: publicClients.ObjectId, + PublicClientsAppName: publicClients.AppName); } private static AddMcpServerRequest BuildRequest(ResolvedInput input, EntraAppSet apps) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppFactory.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppFactory.cs new file mode 100644 index 00000000..85a370c7 --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppFactory.cs @@ -0,0 +1,153 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; +using Microsoft.Extensions.Logging; + +namespace Microsoft.Agents.A365.DevTools.Cli.Services; + +/// +/// Creates the Entra app registrations needed by publish and register flows. Both flows produce +/// the same two app shapes (a confidential proxy app with a secret, and a public-client app with +/// canonical redirect URIs); register additionally produces a second proxy app for the remote +/// MCP server. This factory centralizes the creation steps and their error-handling so the +/// command executors can compose them without duplicating the per-step null checks and logging. +/// +internal class EntraAppFactory +{ + private static readonly string[] PublicClientCanonicalRedirectUris = + [ + "http://localhost:8080/callback", + "https://vscode.dev/redirect", + "http://localhost", + ]; + + private readonly ILogger _logger; + private readonly GraphApiService _graphApiService; + private readonly RetryHelper _retryHelper; + + internal EntraAppFactory(ILogger logger, GraphApiService graphApiService, RetryHelper retryHelper) + { + _logger = logger; + _graphApiService = graphApiService; + _retryHelper = retryHelper; + } + + /// + /// Result of creating a confidential proxy app (A365 Proxy or Remote Proxy). All fields are + /// guaranteed non-empty on success; a null return indicates failure with the cause already + /// logged. + /// + internal sealed record ProxyAppResult(string ClientId, string Secret, string ObjectId, string AppName); + + /// + /// Result of creating the Public Clients app. Failure here is non-fatal: the caller still + /// receives the resolved AppName and a warning is appended to the supplied list, but + /// ClientId/ObjectId may be null if the app or redirect-URI setup failed. + /// + internal sealed record PublicClientsAppResult(string? ClientId, string? ObjectId, string AppName); + + /// + /// Creates a confidential proxy Entra app named {serverName}-{suffix}, adds a client + /// secret, and validates that the returned client ID is non-empty. Used for both the A365 + /// Proxy (publish + register) and the Remote Proxy (register, when auth is EntraOAuth). + /// + internal virtual async Task CreateProxyAppAsync( + string serverName, + string tenantId, + string suffix, + string roleDisplay, + string? serviceTreeId, + CancellationToken ct = default) + { + var appName = $"{serverName}-{suffix}"; + + _logger.LogDebug("Creating Entra application for {Role}...", roleDisplay); + var app = await _graphApiService.CreateEntraAppAsync(tenantId, appName, serviceTreeId: serviceTreeId, ct); + if (app == null) + { + _logger.LogError("Failed to create Entra application '{AppName}'. Ensure you have Application.ReadWrite.All permission in the target tenant. Run with -v for details.", appName); + return null; + } + _logger.LogInformation("Created Entra app '{AppName}' (clientId: {ClientId})", appName, app.Value.ClientId); + + var secret = await _graphApiService.AddAppPasswordAsync(tenantId, app.Value.ObjectId); + if (string.IsNullOrWhiteSpace(secret)) + { + _logger.LogError("Failed to create secret for '{AppName}'. Run with -v for details.", appName); + return null; + } + + if (string.IsNullOrWhiteSpace(app.Value.ClientId)) + { + _logger.LogError("{Role} Entra application was created but returned an empty client ID", roleDisplay); + return null; + } + + _logger.LogDebug("Created {Role} app: {ClientId}", roleDisplay, app.Value.ClientId); + return new ProxyAppResult(app.Value.ClientId, secret, app.Value.ObjectId, appName); + } + + /// + /// Creates the Public Clients Entra app named {serverName}-PublicClients and sets its + /// public-client redirect URIs (broker plugin + canonical localhost/vscode URIs). Best-effort: + /// any failure is converted to a warning rather than failing the caller; the returned record + /// carries null IDs to signal partial success while still surfacing the app name. + /// + internal virtual async Task CreatePublicClientsAppAsync( + string serverName, + string tenantId, + string? serviceTreeId, + List warnings, + CancellationToken ct = default) + { + var appName = $"{serverName}-PublicClients"; + + _logger.LogDebug("Creating Entra application for Public Clients..."); + var app = await _graphApiService.CreateEntraAppAsync(tenantId, appName, serviceTreeId: serviceTreeId, ct); + if (app == null) + { + var msg = "Failed to create Public Clients Entra app. Continuing without it."; + _logger.LogWarning(msg); + warnings.Add(msg); + return new PublicClientsAppResult(null, null, appName); + } + + var clientId = app.Value.ClientId; + var objectId = app.Value.ObjectId; + _logger.LogInformation("Created Entra app '{AppName}' (clientId: {ClientId})", appName, clientId); + + var brokerRedirectUri = $"ms-appx-web://Microsoft.AAD.BrokerPlugin/{clientId}"; + var publicClientUris = new[] { brokerRedirectUri }.Concat(PublicClientCanonicalRedirectUris).ToArray(); + + try + { + var success = await _retryHelper.ExecuteWithRetryAsync( + async retryCt => await _graphApiService.UpdateAppPublicClientRedirectUrisAsync(tenantId, objectId, publicClientUris, retryCt), + result => !result); + if (!success) + { + var msg = $"Failed to set redirect URIs on Public Clients app '{appName}' after retries."; + _logger.LogError(msg); + warnings.Add(msg); + } + else + { + _logger.LogDebug( + "Set {RedirectUriCount} redirect URIs on '{AppName}' ({ObjectId}): {RedirectUris}", + publicClientUris.Length, + appName, + objectId, + string.Join(", ", publicClientUris)); + } + } + catch (Exception ex) + { + var msg = $"Failed to set redirect URIs on Public Clients app: {ex.Message}"; + _logger.LogError(msg); + warnings.Add(msg); + } + + return new PublicClientsAppResult(clientId, objectId, appName); + } +} diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandTests.cs index b28f0979..278dedfa 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandTests.cs @@ -122,15 +122,16 @@ public void PublishSubcommand_HasCorrectOptionsWithAliases() var options = subcommand.Options.ToList(); - // Verify all expected options exist (including tenant-id + service-tree-id added for the Entra - // app creation step that mirrors register-external-mcp-server). + // Verify all expected options exist. Tenant ID is auto-detected from the current az login + // session, so publish does not expose --tenant-id; ServiceTree tagging is not required for + // publish since it targets Dataverse environments rather than Microsoft corp tenants. var optionNames = options.Select(o => o.Name).ToList(); optionNames.Should().Contain("environment-id"); optionNames.Should().Contain("server-name"); optionNames.Should().Contain("alias"); optionNames.Should().Contain("display-name"); - optionNames.Should().Contain("tenant-id"); - optionNames.Should().Contain("service-tree-id"); + optionNames.Should().NotContain("tenant-id"); + optionNames.Should().NotContain("service-tree-id"); optionNames.Should().Contain("dry-run"); // Verify critical aliases for Azure CLI compliance @@ -145,9 +146,6 @@ public void PublishSubcommand_HasCorrectOptionsWithAliases() var displayNameOption = options.FirstOrDefault(o => o.Name == "display-name"); displayNameOption!.Aliases.Should().Contain("-d"); - - var tenantOption = options.FirstOrDefault(o => o.Name == "tenant-id"); - tenantOption!.Aliases.Should().Contain("-t"); } [Fact] @@ -283,7 +281,7 @@ public void RegisterExternalMcpServerSubcommand_HasAllExpectedOptions() optionNames.Should().Contain("tools"); optionNames.Should().Contain("input-file"); optionNames.Should().Contain("remote-scopes"); - optionNames.Should().Contain("tenant-id"); + optionNames.Should().NotContain("tenant-id", because: "tenant ID is auto-detected by RegisterCommandExecutor via TenantDetectionHelper"); optionNames.Should().Contain("service-tree-id"); optionNames.Should().Contain("secret-lifetime-months"); optionNames.Should().Contain("publisher"); @@ -306,9 +304,6 @@ public void RegisterExternalMcpServerSubcommand_HasAllExpectedOptions() var inputFileOption = options.First(o => o.Name == "input-file"); inputFileOption.Aliases.Should().Contain("-f"); - var tenantIdOption = options.First(o => o.Name == "tenant-id"); - tenantIdOption.Aliases.Should().Contain("-t"); - var verboseOption = options.First(o => o.Name == "verbose"); verboseOption.Aliases.Should().Contain("-v"); } @@ -326,7 +321,6 @@ public void RegisterExternalMcpServerSubcommand_HasAllExpectedOptions() [InlineData("register-external-mcp-server", "server-url", "-u")] [InlineData("register-external-mcp-server", "auth-type", "-a")] [InlineData("register-external-mcp-server", "input-file", "-f")] - [InlineData("register-external-mcp-server", "tenant-id", "-t")] [InlineData("register-external-mcp-server", "secret-lifetime-months", "-l")] public void CriticalOptions_HaveConsistentAliases(string subcommandName, string optionName, string expectedAlias) { diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorDryRunTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorDryRunTests.cs new file mode 100644 index 00000000..04e9d85d --- /dev/null +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorDryRunTests.cs @@ -0,0 +1,66 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using FluentAssertions; +using Microsoft.Agents.A365.DevTools.Cli.Commands; +using Microsoft.Agents.A365.DevTools.Cli.Services; +using Microsoft.Extensions.Logging; +using NSubstitute; +using Xunit; + +namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Commands; + +/// +/// Tests for dry-run output. The dry-run log must mirror the +/// real Entra app naming scheme (derived from ServerName) so users can predict what will be +/// created. Earlier the log used Alias, which diverged from CreateEntraAppsAsync's +/// {ServerName}-A365Proxy / {ServerName}-PublicClients naming. +/// +public class PublishCommandExecutorDryRunTests +{ + [Fact] + public async Task ExecuteAsync_DryRun_LogsEntraAppNamesDerivedFromServerName() + { + var logger = Substitute.For(); + var toolingService = Substitute.For(); + + // ServerName and Alias are intentionally distinct so a regression that reverts to Alias + // would surface in the negative assertion below. + const string serverName = "mcp_TestServer"; + const string alias = "myAlias"; + const string expectedA365AppName = "mcp_TestServer-A365Proxy"; + const string expectedPublicClientsAppName = "mcp_TestServer-PublicClients"; + + var args = new RawPublishArgs( + EnvironmentId: "00000000-0000-0000-0000-000000000000", + ServerName: serverName, + Alias: alias, + DisplayName: "Test Display", + DryRun: true); + + var executor = new PublishCommandExecutor(logger, toolingService, graphApiService: null); + + await executor.ExecuteAsync(args, CancellationToken.None); + + logger.Received(1).Log( + LogLevel.Information, + Arg.Any(), + Arg.Is(o => + o.ToString()!.Contains("[DRY RUN] Would create Entra apps") && + o.ToString()!.Contains(expectedA365AppName) && + o.ToString()!.Contains(expectedPublicClientsAppName) && + !o.ToString()!.Contains($"{alias}-A365Proxy") && + !o.ToString()!.Contains($"{alias}-PublicClients")), + Arg.Any(), + Arg.Any>()); + + logger.Received(1).Log( + LogLevel.Information, + Arg.Any(), + Arg.Is(o => o.ToString()!.Contains("Would call publish endpoint and back-fill")), + Arg.Any(), + Arg.Any>()); + + await toolingService.DidNotReceiveWithAnyArgs().PublishServerV2Async(default!, default!, default!, default); + } +} diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppFactoryTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppFactoryTests.cs new file mode 100644 index 00000000..e64adc35 --- /dev/null +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppFactoryTests.cs @@ -0,0 +1,224 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using FluentAssertions; +using Microsoft.Agents.A365.DevTools.Cli.Services; +using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; +using Microsoft.Extensions.Logging; +using NSubstitute; +using Xunit; + +namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Services; + +/// +/// Tests for . The factory absorbed the per-app creation flows that +/// previously lived inline in PublishCommandExecutor and RegisterCommandExecutor. These tests +/// pin every branch (success + each failure mode) so the executors can compose the factory +/// without needing their own per-step coverage. +/// +public class EntraAppFactoryTests +{ + private const string ServerName = "mcp_TestServer"; + private const string TenantId = "00000000-0000-0000-0000-0000000000aa"; + private const string AppObjectId = "11111111-1111-1111-1111-111111111111"; + private const string AppClientId = "22222222-2222-2222-2222-222222222222"; + private const string AppSecret = "fake-secret"; + + private readonly ILogger _logger; + private readonly GraphApiService _graph; + private readonly RetryHelper _retryHelper; + private readonly EntraAppFactory _factory; + + public EntraAppFactoryTests() + { + _logger = Substitute.For(); + _graph = Substitute.For(); + _retryHelper = new RetryHelper(_logger, maxRetries: 1, baseDelaySeconds: 0); + _factory = new EntraAppFactory(_logger, _graph, _retryHelper); + } + + [Fact] + public async Task CreateProxyAppAsync_HappyPath_ReturnsAllFieldsPopulated() + { + _graph.CreateEntraAppAsync(TenantId, $"{ServerName}-A365Proxy", serviceTreeId: "svc-tree-7", Arg.Any()) + .Returns(Task.FromResult<(string ObjectId, string ClientId)?>((AppObjectId, AppClientId))); + _graph.AddAppPasswordAsync(TenantId, AppObjectId).Returns(Task.FromResult(AppSecret)); + + var result = await _factory.CreateProxyAppAsync(ServerName, TenantId, suffix: "A365Proxy", roleDisplay: "A365 Proxy", serviceTreeId: "svc-tree-7"); + + result.Should().NotBeNull(); + result!.ClientId.Should().Be(AppClientId); + result.Secret.Should().Be(AppSecret); + result.ObjectId.Should().Be(AppObjectId); + result.AppName.Should().Be($"{ServerName}-A365Proxy"); + + await _graph.Received(1).CreateEntraAppAsync(TenantId, $"{ServerName}-A365Proxy", serviceTreeId: "svc-tree-7", Arg.Any()); + await _graph.Received(1).AddAppPasswordAsync(TenantId, AppObjectId); + } + + [Fact] + public async Task CreateProxyAppAsync_UsesSuffixInAppName() + { + _graph.CreateEntraAppAsync(TenantId, $"{ServerName}-RemoteProxy", serviceTreeId: null, Arg.Any()) + .Returns(Task.FromResult<(string ObjectId, string ClientId)?>((AppObjectId, AppClientId))); + _graph.AddAppPasswordAsync(TenantId, AppObjectId).Returns(Task.FromResult(AppSecret)); + + var result = await _factory.CreateProxyAppAsync(ServerName, TenantId, suffix: "RemoteProxy", roleDisplay: "Remote Proxy", serviceTreeId: null); + + result.Should().NotBeNull(); + result!.AppName.Should().Be($"{ServerName}-RemoteProxy"); + } + + [Fact] + public async Task CreateProxyAppAsync_WhenCreateAppReturnsNull_ReturnsNullAndLogsError() + { + _graph.CreateEntraAppAsync(TenantId, Arg.Any(), serviceTreeId: Arg.Any(), Arg.Any()) + .Returns(Task.FromResult<(string ObjectId, string ClientId)?>(null)); + + var result = await _factory.CreateProxyAppAsync(ServerName, TenantId, suffix: "A365Proxy", roleDisplay: "A365 Proxy", serviceTreeId: null); + + result.Should().BeNull(); + await _graph.DidNotReceive().AddAppPasswordAsync(Arg.Any(), Arg.Any()); + _logger.Received(1).Log( + LogLevel.Error, + Arg.Any(), + Arg.Is(o => o.ToString()!.Contains("Failed to create Entra application") && o.ToString()!.Contains($"{ServerName}-A365Proxy")), + Arg.Any(), + Arg.Any>()); + } + + [Fact] + public async Task CreateProxyAppAsync_WhenAddPasswordReturnsNull_ReturnsNullAndLogsError() + { + _graph.CreateEntraAppAsync(TenantId, Arg.Any(), serviceTreeId: Arg.Any(), Arg.Any()) + .Returns(Task.FromResult<(string ObjectId, string ClientId)?>((AppObjectId, AppClientId))); + _graph.AddAppPasswordAsync(TenantId, AppObjectId).Returns(Task.FromResult(null)); + + var result = await _factory.CreateProxyAppAsync(ServerName, TenantId, suffix: "A365Proxy", roleDisplay: "A365 Proxy", serviceTreeId: null); + + result.Should().BeNull(); + _logger.Received(1).Log( + LogLevel.Error, + Arg.Any(), + Arg.Is(o => o.ToString()!.Contains("Failed to create secret") && o.ToString()!.Contains($"{ServerName}-A365Proxy")), + Arg.Any(), + Arg.Any>()); + } + + [Fact] + public async Task CreateProxyAppAsync_WhenAddPasswordReturnsWhitespace_ReturnsNullAndLogsError() + { + _graph.CreateEntraAppAsync(TenantId, Arg.Any(), serviceTreeId: Arg.Any(), Arg.Any()) + .Returns(Task.FromResult<(string ObjectId, string ClientId)?>((AppObjectId, AppClientId))); + _graph.AddAppPasswordAsync(TenantId, AppObjectId).Returns(Task.FromResult(" ")); + + var result = await _factory.CreateProxyAppAsync(ServerName, TenantId, suffix: "A365Proxy", roleDisplay: "A365 Proxy", serviceTreeId: null); + + result.Should().BeNull(); + } + + [Fact] + public async Task CreateProxyAppAsync_WhenClientIdIsEmpty_ReturnsNullAndLogsError() + { + _graph.CreateEntraAppAsync(TenantId, Arg.Any(), serviceTreeId: Arg.Any(), Arg.Any()) + .Returns(Task.FromResult<(string ObjectId, string ClientId)?>((AppObjectId, string.Empty))); + _graph.AddAppPasswordAsync(TenantId, AppObjectId).Returns(Task.FromResult(AppSecret)); + + var result = await _factory.CreateProxyAppAsync(ServerName, TenantId, suffix: "RemoteProxy", roleDisplay: "Remote Proxy", serviceTreeId: null); + + result.Should().BeNull(); + _logger.Received(1).Log( + LogLevel.Error, + Arg.Any(), + Arg.Is(o => o.ToString()!.Contains("Remote Proxy") && o.ToString()!.Contains("empty client ID")), + Arg.Any(), + Arg.Any>()); + } + + [Fact] + public async Task CreatePublicClientsAppAsync_HappyPath_ReturnsIdsAndSetsBrokerAndCanonicalRedirectUris() + { + var capturedUris = new List(); + _graph.CreateEntraAppAsync(TenantId, $"{ServerName}-PublicClients", serviceTreeId: "svc-tree-9", Arg.Any()) + .Returns(Task.FromResult<(string ObjectId, string ClientId)?>((AppObjectId, AppClientId))); + _graph.UpdateAppPublicClientRedirectUrisAsync( + TenantId, + AppObjectId, + Arg.Do(uris => capturedUris.Add(uris)), + Arg.Any()) + .Returns(Task.FromResult(true)); + + var warnings = new List(); + var result = await _factory.CreatePublicClientsAppAsync(ServerName, TenantId, serviceTreeId: "svc-tree-9", warnings); + + result.Should().NotBeNull(); + result.ClientId.Should().Be(AppClientId); + result.ObjectId.Should().Be(AppObjectId); + result.AppName.Should().Be($"{ServerName}-PublicClients"); + warnings.Should().BeEmpty(); + + capturedUris.Should().ContainSingle(); + var uris = capturedUris[0]; + uris.Should().BeEquivalentTo(new[] + { + $"ms-appx-web://Microsoft.AAD.BrokerPlugin/{AppClientId}", + "http://localhost:8080/callback", + "https://vscode.dev/redirect", + "http://localhost", + }, opt => opt.WithStrictOrdering()); + } + + [Fact] + public async Task CreatePublicClientsAppAsync_WhenCreateAppReturnsNull_ReturnsAppNameOnlyAndAppendsWarning() + { + _graph.CreateEntraAppAsync(TenantId, Arg.Any(), serviceTreeId: Arg.Any(), Arg.Any()) + .Returns(Task.FromResult<(string ObjectId, string ClientId)?>(null)); + + var warnings = new List(); + var result = await _factory.CreatePublicClientsAppAsync(ServerName, TenantId, serviceTreeId: null, warnings); + + result.ClientId.Should().BeNull(); + result.ObjectId.Should().BeNull(); + result.AppName.Should().Be($"{ServerName}-PublicClients"); + warnings.Should().ContainSingle().Which.Should().Be("Failed to create Public Clients Entra app. Continuing without it."); + + await _graph.DidNotReceive().UpdateAppPublicClientRedirectUrisAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()); + } + + [Fact] + public async Task CreatePublicClientsAppAsync_WhenRedirectUriUpdateReturnsFalse_ReturnsIdsAndAppendsRetryWarning() + { + _graph.CreateEntraAppAsync(TenantId, Arg.Any(), serviceTreeId: Arg.Any(), Arg.Any()) + .Returns(Task.FromResult<(string ObjectId, string ClientId)?>((AppObjectId, AppClientId))); + _graph.UpdateAppPublicClientRedirectUrisAsync( + TenantId, AppObjectId, Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(false)); + + var warnings = new List(); + var result = await _factory.CreatePublicClientsAppAsync(ServerName, TenantId, serviceTreeId: null, warnings); + + result.ClientId.Should().Be(AppClientId); + result.ObjectId.Should().Be(AppObjectId); + result.AppName.Should().Be($"{ServerName}-PublicClients"); + warnings.Should().ContainSingle().Which.Should().Be($"Failed to set redirect URIs on Public Clients app '{ServerName}-PublicClients' after retries."); + } + + [Fact] + public async Task CreatePublicClientsAppAsync_WhenRedirectUriUpdateThrows_ReturnsIdsAndAppendsExceptionWarning() + { + _graph.CreateEntraAppAsync(TenantId, Arg.Any(), serviceTreeId: Arg.Any(), Arg.Any()) + .Returns(Task.FromResult<(string ObjectId, string ClientId)?>((AppObjectId, AppClientId))); + _graph.UpdateAppPublicClientRedirectUrisAsync( + TenantId, AppObjectId, Arg.Any(), Arg.Any()) + .Returns>(_ => throw new InvalidOperationException("Graph blew up")); + + var warnings = new List(); + var result = await _factory.CreatePublicClientsAppAsync(ServerName, TenantId, serviceTreeId: null, warnings); + + result.ClientId.Should().Be(AppClientId); + result.ObjectId.Should().Be(AppObjectId); + result.AppName.Should().Be($"{ServerName}-PublicClients"); + warnings.Should().ContainSingle().Which.Should().Be("Failed to set redirect URIs on Public Clients app: Graph blew up"); + } +} From 52ef0cd4d936abaab111786296dd51636124fa16 Mon Sep 17 00:00:00 2001 From: Deepali Garg Date: Thu, 28 May 2026 13:28:33 -0700 Subject: [PATCH 05/12] Commenting out A365 proxy app --- .../Commands/PublishCommandExecutor.cs | 68 +++++++++++++++---- .../PublishCommandExecutorDryRunTests.cs | 55 ++++++++++++++- 2 files changed, 110 insertions(+), 13 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs index 30caab87..890fe196 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs @@ -21,10 +21,13 @@ internal record RawPublishArgs( /// /// Orchestrates first-party MCP server publish in one CLI command. The shape mirrors -/// for BYO register: the CLI creates the Entra apps it has -/// delegated authority to create (A365 Proxy + Public Clients in the user's own tenant), calls the -/// platform publish endpoint with those credentials, and back-fills the apps' redirect URIs and -/// PPMI scope grants after the platform creates the CMS connector and PPMI identity. +/// for BYO register: the CLI creates the Public Clients +/// Entra app in the user's own tenant, calls the platform publish endpoint, and back-fills the +/// PPMI scope grant on it after the platform resolves the underlying server's PPMI identity. +/// NOTE: A365 Proxy Entra app creation + redirect-URI back-fill are TEMPORARILY DISABLED while +/// the platform-side custom connector flow is commented out (see PublishMCPServerV2Async in +/// MCPDataverseEnvironmentService). Reinstate the proxy-related blocks below together with the +/// platform-side flow. /// internal class PublishCommandExecutor { @@ -71,8 +74,15 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct = def if (input.DryRun) { + // TEMPORARILY DISABLED: A365 Proxy dry-run lines (proxy app creation + redirect URI + // back-fill). Reinstate together with the corresponding logic in CreateEntraAppsAsync + // and ConfigureEntraAppsAsync. + /* _logger.LogInformation("[DRY RUN] Would create Entra apps '{A365}' and '{PublicClients}' in tenant", $"{input.ServerName}-A365Proxy", $"{input.ServerName}-PublicClients"); _logger.LogInformation("[DRY RUN] Would call publish endpoint and back-fill redirect URI + PPMI scope on the created apps"); + */ + _logger.LogInformation("[DRY RUN] Would create Entra app '{PublicClients}' in tenant", $"{input.ServerName}-PublicClients"); + _logger.LogInformation("[DRY RUN] Would call publish endpoint and back-fill PPMI scope on the created app"); return; } @@ -103,19 +113,26 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct = def ct.ThrowIfCancellationRequested(); + // TEMPORARILY DISABLED: A365 Proxy client-id parse + creds assignment to the publish request. + // The platform's v2 publish ignores these fields while the custom connector flow is + // disabled (see PublishMCPServerV2Async Step 3 comment block in MCPDataverseEnvironmentService). + // Reinstate the parse block, the apps == null short-circuit above, and the two request + // fields together when the proxy flow returns. + /* if (!Guid.TryParse(apps.A365AppClientId, out var a365ProxyClientId)) { _logger.LogError("A365 Proxy Entra app returned an invalid client ID '{ClientId}'. Expected a GUID. Cannot continue publish.", apps.A365AppClientId); await RollbackEntraAppsAsync(apps, tenantId, ct); return; } + */ var request = new PublishMcpServerRequest { Alias = input.Alias, DisplayName = input.DisplayName, - A365ProxyClientId = a365ProxyClientId, - A365ProxyClientSecret = apps.A365AppSecret, + // A365ProxyClientId = a365ProxyClientId, + // A365ProxyClientSecret = apps.A365AppSecret, PublicClientsAppId = apps.PublicClientsClientId, }; @@ -123,8 +140,10 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct = def try { // Hits the v2 publish endpoint, which performs the full elevation orchestration - // (lazy PPMI, MOS upload, A365 Proxy CMS connector creation). v1's PublishServerAsync - // is preserved on the platform for any callers relying on the original behavior. + // (PPMI, MOS upload). A365 Proxy CMS connector creation is TEMPORARILY DISABLED + // on the platform side (see PublishMCPServerV2Async Step 3 comment block) while the + // custom connector flow is being re-evaluated. v1's PublishServerAsync is preserved on + // the platform for any callers relying on the original behavior. publishResponse = await _toolingService.PublishServerV2Async(input.EnvironmentId, input.ServerName, request, ct); } catch (Exception ex) @@ -250,18 +269,24 @@ private void DisplayPublishSummary(ResolvedInput input) { var factory = new EntraAppFactory(_logger, _graphApiService!, _retryHelper); + // TEMPORARILY DISABLED: A365 Proxy Entra app creation. The platform-side custom connector + // flow that consumed these credentials is commented out, so creating the app here would + // leak an unused Entra registration in the user's tenant. Reinstate together with the + // platform flow. A365App* fields on the returned EntraAppSet are placeholder empties. + /* var a365 = await factory.CreateProxyAppAsync( input.ServerName, tenantId, suffix: "A365Proxy", roleDisplay: "A365 Proxy", serviceTreeId: null); if (a365 == null) return null; + */ var publicClients = await factory.CreatePublicClientsAppAsync( input.ServerName, tenantId, serviceTreeId: null, warnings); return new EntraAppSet( - A365AppClientId: a365.ClientId, - A365AppSecret: a365.Secret, - A365AppObjectId: a365.ObjectId, - A365AppName: a365.AppName, + A365AppClientId: string.Empty, + A365AppSecret: string.Empty, + A365AppObjectId: string.Empty, + A365AppName: string.Empty, PublicClientsClientId: publicClients.ClientId, PublicClientsObjectId: publicClients.ObjectId, PublicClientsAppName: publicClients.AppName); @@ -281,7 +306,11 @@ internal async Task RollbackEntraAppsAsync(EntraAppSet apps, string tenantId, Ca _logger.LogInformation("Rolling back Entra app registrations created for failed publish..."); + // TEMPORARILY DISABLED: A365 Proxy delete. The proxy Entra app is no longer created + // (see CreateEntraAppsAsync). Reinstate together with the proxy app creation. + /* await DeleteOneAsync(apps.A365AppObjectId, apps.A365AppClientId, apps.A365AppName, ct); + */ if (!string.IsNullOrWhiteSpace(apps.PublicClientsObjectId)) { @@ -325,6 +354,11 @@ private async Task ConfigureEntraAppsAsync( var tasks = new List(); var concurrentWarnings = new System.Collections.Concurrent.ConcurrentBag(); + // TEMPORARILY DISABLED: A365 Proxy redirect-URI back-fill. The platform's v2 publish no + // longer creates the proxy connector, so A365ProxyRedirectUri always comes back null and + // there is no A365 Proxy Entra app to write a redirect URI onto. Reinstate together with + // the proxy app creation in CreateEntraAppsAsync. + /* var a365RedirectUri = response.A365ProxyRedirectUri; if (!string.IsNullOrWhiteSpace(a365RedirectUri)) @@ -337,6 +371,7 @@ private async Task ConfigureEntraAppsAsync( _logger.LogWarning(msg); concurrentWarnings.Add(msg); } + */ // Grant required-resource-access on the just-created A365 Proxy + Public Clients Entra apps. // The platform resolves the right resource per server type (Custom: managedidentityid; app-based @@ -373,7 +408,12 @@ private async Task ConfigureEntraAppsAsync( if (resourceScopeId.HasValue) { + // TEMPORARILY DISABLED: required-resource-access grant on the A365 Proxy app. The proxy + // app is no longer created (see CreateEntraAppsAsync). Reinstate together with the + // proxy app creation. + /* tasks.Add(AddRequiredResourceAccessAsync(tenantId, apps.A365AppObjectId, apps.A365AppName, resourceAppId!, resourceScopeId.Value, concurrentWarnings, ct)); + */ if (apps.PublicClientsObjectId != null) { @@ -393,6 +433,9 @@ private async Task ConfigureEntraAppsAsync( warnings.Add(w); } + // TEMPORARILY DISABLED: UpdateA365RedirectUrisAsync. Sole caller in ConfigureEntraAppsAsync is + // commented out. Reinstate together with the A365 Proxy app creation in CreateEntraAppsAsync. + /* private async Task UpdateA365RedirectUrisAsync( string tenantId, EntraAppSet apps, @@ -428,6 +471,7 @@ private async Task UpdateA365RedirectUrisAsync( concurrentWarnings.Add(msg); } } + */ private async Task AddRequiredResourceAccessAsync( string tenantId, diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorDryRunTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorDryRunTests.cs index 04e9d85d..3db37d5c 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorDryRunTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorDryRunTests.cs @@ -18,7 +18,7 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Commands; /// public class PublishCommandExecutorDryRunTests { - [Fact] + [Fact(Skip = "A365 Proxy Entra app creation is TEMPORARILY DISABLED in PublishCommandExecutor while the platform custom connector flow is commented out. Dry-run log no longer mentions the A365Proxy app or the redirect-URI back-fill. Re-enable together with CreateEntraAppsAsync's proxy creation.")] public async Task ExecuteAsync_DryRun_LogsEntraAppNamesDerivedFromServerName() { var logger = Substitute.For(); @@ -63,4 +63,57 @@ public async Task ExecuteAsync_DryRun_LogsEntraAppNamesDerivedFromServerName() await toolingService.DidNotReceiveWithAnyArgs().PublishServerV2Async(default!, default!, default!, default); } + + /// + /// While the A365 Proxy flow is disabled, the dry-run log must (a) mention only the Public + /// Clients app — not the A365 Proxy app — derived from ServerName, (b) replace the + /// redirect-URI back-fill line with a PPMI-scope-only back-fill line, and (c) still skip the + /// platform publish call. Acts as the canary: if the proxy line silently comes back without + /// the corresponding logic being re-enabled, this fails. + /// + [Fact] + public async Task ExecuteAsync_DryRun_OmitsA365ProxyApp_WhileCustomConnectorFlowDisabled() + { + var logger = Substitute.For(); + var toolingService = Substitute.For(); + + const string serverName = "mcp_TestServer"; + const string alias = "myAlias"; + const string disabledA365AppName = "mcp_TestServer-A365Proxy"; + const string expectedPublicClientsAppName = "mcp_TestServer-PublicClients"; + + var args = new RawPublishArgs( + EnvironmentId: "00000000-0000-0000-0000-000000000000", + ServerName: serverName, + Alias: alias, + DisplayName: "Test Display", + DryRun: true); + + var executor = new PublishCommandExecutor(logger, toolingService, graphApiService: null); + + await executor.ExecuteAsync(args, CancellationToken.None); + + // The active dry-run line names only Public Clients and does NOT name A365 Proxy. + logger.Received(1).Log( + LogLevel.Information, + Arg.Any(), + Arg.Is(o => + o.ToString()!.Contains("[DRY RUN] Would create Entra app") && + o.ToString()!.Contains(expectedPublicClientsAppName) && + !o.ToString()!.Contains(disabledA365AppName)), + Arg.Any(), + Arg.Any>()); + + // The back-fill line now mentions only PPMI scope, not redirect URI. + logger.Received(1).Log( + LogLevel.Information, + Arg.Any(), + Arg.Is(o => + o.ToString()!.Contains("back-fill PPMI scope") && + !o.ToString()!.Contains("redirect URI")), + Arg.Any(), + Arg.Any>()); + + await toolingService.DidNotReceiveWithAnyArgs().PublishServerV2Async(default!, default!, default!, default); + } } From 55b5285ba08a920d12134c081c25022ffc944192 Mon Sep 17 00:00:00 2001 From: Deepali Garg Date: Sun, 31 May 2026 22:27:06 -0700 Subject: [PATCH 06/12] Adding publisher parameter --- .../Commands/DevelopMcpCommand.cs | 6 ++++ .../Commands/PublishCommandExecutor.cs | 31 ++++++++++++++++++ .../Models/PublishMcpServerRequest.cs | 9 ++++++ .../PublishCommandExecutorDryRunTests.cs | 32 +++++++++++++++++++ 4 files changed, 78 insertions(+) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs index acc6dac0..dae869f5 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs @@ -306,6 +306,11 @@ private static Command CreatePublishSubcommand( description: "Display name for the MCP server (max 30 chars)"); command.AddOption(displayNameOption); + var publisherNameOption = new Option( + ["--publisher-name", "-p"], + description: "Publisher name written into the MOS package manifest. Required for custom (user-created) MCP servers; ignored for 1p Microsoft-owned servers (e.g. msdyn_DataverseMCPServer) which always publish as 'Microsoft'."); + command.AddOption(publisherNameOption); + var dryRunOption = new Option("--dry-run", "Show what would be done without executing"); command.AddOption(dryRunOption); @@ -319,6 +324,7 @@ private static Command CreatePublishSubcommand( ServerName: context.ParseResult.GetValueForOption(serverNameOption), Alias: context.ParseResult.GetValueForOption(aliasOption), DisplayName: context.ParseResult.GetValueForOption(displayNameOption), + PublisherName: context.ParseResult.GetValueForOption(publisherNameOption), DryRun: context.ParseResult.GetValueForOption(dryRunOption)); var executor = new PublishCommandExecutor(logger, toolingService, graphApiService); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs index 890fe196..7dba9ed4 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs @@ -17,6 +17,7 @@ internal record RawPublishArgs( string? ServerName, string? Alias, string? DisplayName, + string? PublisherName, bool DryRun); /// @@ -54,6 +55,13 @@ private sealed record ResolvedInput public required string Alias { get; init; } public required string DisplayName { get; init; } public required bool DryRun { get; init; } + + // Null when the caller didn't supply --publisher-name and didn't enter one at the prompt. + // The platform's v2 publish validator rejects null/empty for custom (user-created) servers + // and ignores the value for 1p app-based servers (auto-fills "Microsoft"). The CLI can't + // classify ahead of time without knowing the server's mapping, so it leaves the value + // null when unspecified and lets the platform decide. + public string? PublisherName { get; init; } } internal sealed record EntraAppSet( @@ -134,6 +142,7 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct = def // A365ProxyClientId = a365ProxyClientId, // A365ProxyClientSecret = apps.A365AppSecret, PublicClientsAppId = apps.PublicClientsClientId, + PublisherName = input.PublisherName, }; PublishMcpServerResponse? publishResponse; @@ -221,12 +230,33 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct = def if (displayName == null) { _logger.LogError("Invalid display name format"); return null; } } + // Publisher name: optional from the CLI's perspective. The platform's v2 validator + // requires a non-empty value for custom (user-created) servers and ignores it for + // 1p app-based servers. Prompt the user when not supplied, but allow empty input — + // a Microsoft developer publishing msdyn_DataverseMCPServer shouldn't have to type + // anything. If they're publishing a custom server with no value, the platform's + // error message tells them what's missing. + var publisherName = args.PublisherName; + if (string.IsNullOrWhiteSpace(publisherName)) + { + publisherName = DevelopMcpCommand.InputValidator.PromptAndValidateOptionalInput( + "Enter publisher name (optional for 1p Microsoft-owned servers, required otherwise): ", + "Publisher name", + maxLength: 100); + } + else + { + publisherName = DevelopMcpCommand.InputValidator.ValidateInput(publisherName, "Publisher name", maxLength: 100); + if (publisherName == null) { _logger.LogError("Invalid publisher name format"); return null; } + } + return new ResolvedInput { EnvironmentId = environmentId, ServerName = serverName, Alias = alias, DisplayName = displayName, + PublisherName = string.IsNullOrWhiteSpace(publisherName) ? null : publisherName, DryRun = args.DryRun, }; } @@ -249,6 +279,7 @@ private void DisplayPublishSummary(ResolvedInput input) DevelopMcpCommand.WriteLabel(" Server Name: "); Console.WriteLine(input.ServerName); DevelopMcpCommand.WriteLabel(" Alias: "); Console.WriteLine(input.Alias); DevelopMcpCommand.WriteLabel(" Display Name: "); Console.WriteLine(input.DisplayName); + DevelopMcpCommand.WriteLabel(" Publisher: "); Console.WriteLine(input.PublisherName ?? "(none — platform will reject if this is a custom server)"); Console.WriteLine(); } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerRequest.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerRequest.cs index 67af70da..96924387 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerRequest.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerRequest.cs @@ -43,4 +43,13 @@ public class PublishMcpServerRequest /// [JsonPropertyName("publicClientsAppId")] public string? PublicClientsAppId { get; set; } + + /// + /// Publisher / developer name written into the MOS package manifest. Required for Custom + /// (user-created) servers — the platform's v2 publish validator rejects empty values for those. + /// Ignored for 1p Microsoft-owned app-based servers (e.g. msdyn_DataverseMCPServer), + /// which always publish under "Microsoft" regardless. + /// + [JsonPropertyName("publisherName")] + public string? PublisherName { get; set; } } diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorDryRunTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorDryRunTests.cs index 3db37d5c..f2a9d77f 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorDryRunTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorDryRunTests.cs @@ -36,6 +36,7 @@ public async Task ExecuteAsync_DryRun_LogsEntraAppNamesDerivedFromServerName() ServerName: serverName, Alias: alias, DisplayName: "Test Display", + PublisherName: null, DryRun: true); var executor = new PublishCommandExecutor(logger, toolingService, graphApiService: null); @@ -87,6 +88,7 @@ public async Task ExecuteAsync_DryRun_OmitsA365ProxyApp_WhileCustomConnectorFlow ServerName: serverName, Alias: alias, DisplayName: "Test Display", + PublisherName: null, DryRun: true); var executor = new PublishCommandExecutor(logger, toolingService, graphApiService: null); @@ -116,4 +118,34 @@ public async Task ExecuteAsync_DryRun_OmitsA365ProxyApp_WhileCustomConnectorFlow await toolingService.DidNotReceiveWithAnyArgs().PublishServerV2Async(default!, default!, default!, default); } + + /// + /// Sanity check that --publisher-name threads through the executor without crashing the + /// dry-run flow. The platform's v2 validator rejects an empty publisher only for Custom servers; + /// at this CLI layer we don't classify, we just forward what was provided. This test pins that + /// the field is accepted on , no interactive prompt is invoked when + /// a value is supplied (the prompt would hang in xUnit without stdin), and the dry-run still + /// short-circuits before any platform call. + /// + [Fact] + public async Task ExecuteAsync_DryRun_AcceptsPublisherName_WithoutCallingPlatform() + { + var logger = Substitute.For(); + var toolingService = Substitute.For(); + + var args = new RawPublishArgs( + EnvironmentId: "00000000-0000-0000-0000-000000000000", + ServerName: "mcp_TestServer", + Alias: "myAlias", + DisplayName: "Test Display", + PublisherName: "Contoso", + DryRun: true); + + var executor = new PublishCommandExecutor(logger, toolingService, graphApiService: null); + + await executor.ExecuteAsync(args, CancellationToken.None); + + // Dry-run short-circuits before the platform publish call regardless of publisher. + await toolingService.DidNotReceiveWithAnyArgs().PublishServerV2Async(default!, default!, default!, default); + } } From dd286f67f0239d1955c0d3c92fd15e560c74d5f8 Mon Sep 17 00:00:00 2001 From: Deepali Garg Date: Mon, 1 Jun 2026 17:14:54 -0700 Subject: [PATCH 07/12] Addressing comments --- .../Commands/PublishCommandExecutor.cs | 12 +-- .../Services/Agent365ToolingService.cs | 92 +------------------ .../Services/IAgent365ToolingService.cs | 24 +---- .../PublishCommandExecutorDryRunTests.cs | 6 +- 4 files changed, 16 insertions(+), 118 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs index 7dba9ed4..9d8ad702 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs @@ -148,12 +148,12 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct = def PublishMcpServerResponse? publishResponse; try { - // Hits the v2 publish endpoint, which performs the full elevation orchestration - // (PPMI, MOS upload). A365 Proxy CMS connector creation is TEMPORARILY DISABLED - // on the platform side (see PublishMCPServerV2Async Step 3 comment block) while the - // custom connector flow is being re-evaluated. v1's PublishServerAsync is preserved on - // the platform for any callers relying on the original behavior. - publishResponse = await _toolingService.PublishServerV2Async(input.EnvironmentId, input.ServerName, request, ct); + // Hits the platform's v2 publish endpoint via the tooling service, which performs the + // full elevation orchestration (PPMI provisioning, MOS upload). A365 Proxy CMS + // connector creation is TEMPORARILY DISABLED on the platform side (see + // PublishMCPServerV2Async Step 3 comment block) while the custom connector flow is + // being re-evaluated. The platform's v1 endpoint remains for older CLI binaries. + publishResponse = await _toolingService.PublishServerAsync(input.EnvironmentId, input.ServerName, request, ct); } catch (Exception ex) { diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Agent365ToolingService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Agent365ToolingService.cs index b02cc0f8..f57bfa76 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Agent365ToolingService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Agent365ToolingService.cs @@ -251,27 +251,15 @@ private string BuildListMcpServersUrl(string environment, string environmentId) } /// - /// Builds URL for publishing an MCP server to a Dataverse environment (v1) + /// Builds URL for publishing an MCP server to a Dataverse environment. Hits the platform's v2 + /// publish endpoint, which performs the full elevation orchestration (PPMI provisioning and MOS + /// upload). /// /// Environment name /// Dataverse environment ID /// MCP server name /// Full URL for publish MCP server endpoint private string BuildPublishMcpServerUrl(string environment, string environmentId, string serverName) - { - var baseUrl = BuildAgent365ToolsBaseUrl(environment); - return $"{baseUrl}/agents/dataverse/environments/{environmentId}/mcpServers/{serverName}/publish"; - } - - /// - /// Builds URL for the v2 publish endpoint, which performs the full elevation orchestration - /// (lazy PPMI, MOS upload, A365 Proxy CMS connector creation). - /// - /// Environment name - /// Dataverse environment ID - /// MCP server name - /// Full URL for v2 publish MCP server endpoint - private string BuildPublishMcpServerV2Url(string environment, string environmentId, string serverName) { var baseUrl = BuildAgent365ToolsBaseUrl(environment); return $"{baseUrl}/agents/dataverse/environments/{environmentId}/mcpServers/{serverName}/publish/v2"; @@ -595,80 +583,6 @@ private string BuildGetMCPServerUrl(string environment) } } - /// - public async Task PublishServerV2Async( - string environmentId, - string serverName, - PublishMcpServerRequest request, - CancellationToken cancellationToken = default) - { - if (string.IsNullOrWhiteSpace(environmentId)) - throw new ArgumentException("Environment ID cannot be null or empty", nameof(environmentId)); - if (string.IsNullOrWhiteSpace(serverName)) - throw new ArgumentException("Server name cannot be null or empty", nameof(serverName)); - if (request == null) - throw new ArgumentNullException(nameof(request)); - - try - { - var endpointUrl = BuildPublishMcpServerV2Url(_environment, environmentId, serverName); - var correlationId = Internal.HttpClientFactory.GenerateCorrelationId(); - - _logger.LogDebug("Publishing (v2) MCP server {ServerName} to environment {EnvId} (CorrelationId: {CorrelationId})", serverName, environmentId, correlationId); - _logger.LogDebug("Environment: {Env}", _environment); - _logger.LogDebug("Endpoint URL: {Url}", endpointUrl); - - var audience = ConfigConstants.GetAgent365ToolsResourceAppId(_environment); - _logger.LogDebug("Acquiring access token for audience: {Audience}", audience); - - var loginHint = await AzCliHelper.ResolveLoginHintAsync(); - var authToken = await _authService.GetAccessTokenAsync(audience, userId: loginHint, ct: cancellationToken); - if (string.IsNullOrWhiteSpace(authToken)) - { - _logger.LogError("Failed to acquire authentication token"); - return null; - } - - using var httpClient = Internal.HttpClientFactory.CreateAuthenticatedClient(authToken, correlationId: correlationId); - - var requestPayload = JsonSerializer.Serialize(request); - var jsonContent = new StringContent(requestPayload, System.Text.Encoding.UTF8, "application/json"); - - LogRequest("POST", endpointUrl, requestPayload); - - using var response = await httpClient.PostAsync(endpointUrl, jsonContent, cancellationToken); - - var (isSuccess, responseContent) = await ValidateResponseAsync(response, "publish (v2) MCP server", cancellationToken); - if (!isSuccess) - { - return null; - } - - if (string.IsNullOrWhiteSpace(responseContent)) - { - return new PublishMcpServerResponse - { - Status = "Success", - Message = $"Successfully published {serverName}", - }; - } - - var publishResponse = JsonDeserializationHelper.DeserializeWithDoubleSerialization( - responseContent, _logger); - - return publishResponse ?? new PublishMcpServerResponse - { - Status = "Success", - Message = $"Successfully published {serverName}", - }; - } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to publish (v2) MCP server {ServerName} to environment {EnvId}", serverName, environmentId); - return null; - } - } - /// public async Task UnpublishServerAsync( string environmentId, diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/IAgent365ToolingService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/IAgent365ToolingService.cs index 30103ab3..0460f2b5 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/IAgent365ToolingService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/IAgent365ToolingService.cs @@ -33,36 +33,20 @@ public interface IAgent365ToolingService CancellationToken cancellationToken = default); /// - /// Publishes an MCP server to a Dataverse environment + /// Publishes an MCP server to a Dataverse environment via the platform's v2 publish endpoint, + /// which performs the full elevation orchestration (PPMI provisioning and MOS upload). /// /// Dataverse environment ID /// MCP server name to publish - /// Publish request with alias, display name, and description + /// Publish request with alias, display name, optional Entra app credentials, and optional publisher name /// Cancellation token - /// Response from the publish operation + /// Response from the publish operation, including the underlying server's PPMI app id and redirect URI Task PublishServerAsync( string environmentId, string serverName, PublishMcpServerRequest request, CancellationToken cancellationToken = default); - /// - /// Publishes an MCP server via the v2 endpoint, which performs the full elevation orchestration - /// (lazy PPMI provision, MOS upload, A365 Proxy CMS connector creation when Entra creds are - /// supplied in the request). v1 remains for callers relying on - /// the original side-effect-free behavior. - /// - /// Dataverse environment ID - /// MCP server name to publish - /// Publish request with alias, display name, and optional Entra app credentials - /// Cancellation token - /// Response from the publish operation, including PPMI app id, A365 Proxy connector id, and redirect URI - Task PublishServerV2Async( - string environmentId, - string serverName, - PublishMcpServerRequest request, - CancellationToken cancellationToken = default); - /// /// Unpublishes an MCP server from a Dataverse environment /// diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorDryRunTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorDryRunTests.cs index f2a9d77f..87181b02 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorDryRunTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorDryRunTests.cs @@ -62,7 +62,7 @@ public async Task ExecuteAsync_DryRun_LogsEntraAppNamesDerivedFromServerName() Arg.Any(), Arg.Any>()); - await toolingService.DidNotReceiveWithAnyArgs().PublishServerV2Async(default!, default!, default!, default); + await toolingService.DidNotReceiveWithAnyArgs().PublishServerAsync(default!, default!, default!, default); } /// @@ -116,7 +116,7 @@ public async Task ExecuteAsync_DryRun_OmitsA365ProxyApp_WhileCustomConnectorFlow Arg.Any(), Arg.Any>()); - await toolingService.DidNotReceiveWithAnyArgs().PublishServerV2Async(default!, default!, default!, default); + await toolingService.DidNotReceiveWithAnyArgs().PublishServerAsync(default!, default!, default!, default); } /// @@ -146,6 +146,6 @@ public async Task ExecuteAsync_DryRun_AcceptsPublisherName_WithoutCallingPlatfor await executor.ExecuteAsync(args, CancellationToken.None); // Dry-run short-circuits before the platform publish call regardless of publisher. - await toolingService.DidNotReceiveWithAnyArgs().PublishServerV2Async(default!, default!, default!, default); + await toolingService.DidNotReceiveWithAnyArgs().PublishServerAsync(default!, default!, default!, default); } } From 97450bee617f2404af7dec78ccdce94c7e76ba35 Mon Sep 17 00:00:00 2001 From: Deepali Garg Date: Mon, 1 Jun 2026 17:28:33 -0700 Subject: [PATCH 08/12] Addrssing copilot comments --- .../Commands/DevelopMcpCommand.cs | 6 +- .../Commands/PublishCommandExecutor.cs | 65 ++++++++++++------- .../Services/EntraAppFactory.cs | 48 +++++++++++++- .../Commands/DevelopMcpCommandTests.cs | 21 ++++-- .../Services/EntraAppFactoryTests.cs | 21 ++++-- 5 files changed, 122 insertions(+), 39 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs index dae869f5..28e89c37 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs @@ -328,7 +328,11 @@ private static Command CreatePublishSubcommand( DryRun: context.ParseResult.GetValueForOption(dryRunOption)); var executor = new PublishCommandExecutor(logger, toolingService, graphApiService); - await executor.ExecuteAsync(args, context.GetCancellationToken()); + var success = await executor.ExecuteAsync(args, context.GetCancellationToken()); + if (!success) + { + context.ExitCode = 1; + } }); return command; diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs index 9d8ad702..ff92fd5a 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs @@ -73,10 +73,10 @@ internal sealed record EntraAppSet( string? PublicClientsObjectId, string PublicClientsAppName); - internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct = default) + internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct = default) { var input = ResolveInputs(args); - if (input is null) return; + if (input is null) return false; DisplayPublishSummary(input); @@ -91,7 +91,7 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct = def */ _logger.LogInformation("[DRY RUN] Would create Entra app '{PublicClients}' in tenant", $"{input.ServerName}-PublicClients"); _logger.LogInformation("[DRY RUN] Would call publish endpoint and back-fill PPMI scope on the created app"); - return; + return true; } Console.Write("Proceed with publish? (y/N): "); @@ -99,7 +99,9 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct = def if (confirmation != "y" && confirmation != "yes") { Console.WriteLine("Publish cancelled."); - return; + // User cancellation is not a failure — exit 0. Matches the register command's same + // prompt-cancel path. + return true; } Console.WriteLine(); @@ -107,17 +109,17 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct = def Console.WriteLine($"Publishing MCP server '{input.ServerName}' as '{input.Alias}' to environment {input.EnvironmentId}..."); var tenantId = await DetectTenantIdAsync(); - if (tenantId is null) return; + if (tenantId is null) return false; if (_graphApiService is null) { _logger.LogError("Graph API service is not available. Cannot create Entra applications."); - return; + return false; } var warnings = new List(); - var apps = await CreateEntraAppsAsync(input, tenantId, warnings); - if (apps is null) return; + var apps = await CreateEntraAppsAsync(input, tenantId, warnings, ct); + if (apps is null) return false; ct.ThrowIfCancellationRequested(); @@ -131,7 +133,7 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct = def { _logger.LogError("A365 Proxy Entra app returned an invalid client ID '{ClientId}'. Expected a GUID. Cannot continue publish.", apps.A365AppClientId); await RollbackEntraAppsAsync(apps, tenantId, ct); - return; + return false; } */ @@ -160,7 +162,7 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct = def _logger.LogError("Failed to publish MCP server '{ServerName}': {Error}", input.ServerName, ex.Message); _logger.LogDebug("Exception details: {Exception}", ex.ToString()); await RollbackEntraAppsAsync(apps, tenantId, ct); - return; + return false; } if (publishResponse is null || !publishResponse.IsSuccess) @@ -168,7 +170,7 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct = def var errorMsg = publishResponse?.Message ?? "No response received"; _logger.LogError("Failed to publish MCP server {ServerName}: {Error}", input.ServerName, errorMsg); await RollbackEntraAppsAsync(apps, tenantId, ct); - return; + return false; } _logger.LogDebug("Successfully published MCP server {ServerName}", input.ServerName); @@ -176,16 +178,25 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct = def await ConfigureEntraAppsAsync(input, apps, publishResponse, tenantId, warnings, ct); DisplayResults(input, warnings); + return true; } private ResolvedInput? ResolveInputs(RawPublishArgs args) { + // Dry-run skips interactive prompts so the command stays scriptable / CI-friendly. + // Missing required values get a clearly-labeled placeholder for summary purposes; the + // executor short-circuits before any platform call, so the placeholders never leave + // this process. User-supplied values still go through normal validation. + const string DryRunPlaceholder = "(unspecified)"; + try { var environmentId = args.EnvironmentId; if (string.IsNullOrWhiteSpace(environmentId)) { - environmentId = DevelopMcpCommand.InputValidator.PromptAndValidateRequiredInput("Enter Dataverse environment ID: ", "Environment ID"); + environmentId = args.DryRun + ? DryRunPlaceholder + : DevelopMcpCommand.InputValidator.PromptAndValidateRequiredInput("Enter Dataverse environment ID: ", "Environment ID"); if (string.IsNullOrWhiteSpace(environmentId)) { _logger.LogError("Environment ID is required"); return null; } } else @@ -197,7 +208,9 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct = def var serverName = args.ServerName; if (string.IsNullOrWhiteSpace(serverName)) { - serverName = DevelopMcpCommand.InputValidator.PromptAndValidateRequiredInput("Enter MCP server name to publish: ", "Server name", 100); + serverName = args.DryRun + ? DryRunPlaceholder + : DevelopMcpCommand.InputValidator.PromptAndValidateRequiredInput("Enter MCP server name to publish: ", "Server name", 100); if (string.IsNullOrWhiteSpace(serverName)) { _logger.LogError("Server name is required"); return null; } } else @@ -209,7 +222,9 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct = def var alias = args.Alias; if (string.IsNullOrWhiteSpace(alias)) { - alias = DevelopMcpCommand.InputValidator.PromptAndValidateRequiredInput("Enter alias for the MCP server: ", "Alias", 50); + alias = args.DryRun + ? DryRunPlaceholder + : DevelopMcpCommand.InputValidator.PromptAndValidateRequiredInput("Enter alias for the MCP server: ", "Alias", 50); if (string.IsNullOrWhiteSpace(alias)) { _logger.LogError("Alias is required"); return null; } } else @@ -221,7 +236,9 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct = def var displayName = args.DisplayName; if (string.IsNullOrWhiteSpace(displayName)) { - displayName = DevelopMcpCommand.InputValidator.PromptAndValidateRequiredInput("Enter display name for the MCP server: ", "Display name", 30); + displayName = args.DryRun + ? DryRunPlaceholder + : DevelopMcpCommand.InputValidator.PromptAndValidateRequiredInput("Enter display name for the MCP server: ", "Display name", 30); if (string.IsNullOrWhiteSpace(displayName)) { _logger.LogError("Display name is required"); return null; } } else @@ -235,14 +252,16 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct = def // 1p app-based servers. Prompt the user when not supplied, but allow empty input — // a Microsoft developer publishing msdyn_DataverseMCPServer shouldn't have to type // anything. If they're publishing a custom server with no value, the platform's - // error message tells them what's missing. + // error message tells them what's missing. Dry-run also skips the prompt. var publisherName = args.PublisherName; if (string.IsNullOrWhiteSpace(publisherName)) { - publisherName = DevelopMcpCommand.InputValidator.PromptAndValidateOptionalInput( - "Enter publisher name (optional for 1p Microsoft-owned servers, required otherwise): ", - "Publisher name", - maxLength: 100); + publisherName = args.DryRun + ? null + : DevelopMcpCommand.InputValidator.PromptAndValidateOptionalInput( + "Enter publisher name (optional for 1p Microsoft-owned servers, required otherwise): ", + "Publisher name", + maxLength: 100); } else { @@ -296,7 +315,7 @@ private void DisplayPublishSummary(ResolvedInput input) return tenantId; } - private async Task CreateEntraAppsAsync(ResolvedInput input, string tenantId, List warnings) + private async Task CreateEntraAppsAsync(ResolvedInput input, string tenantId, List warnings, CancellationToken ct = default) { var factory = new EntraAppFactory(_logger, _graphApiService!, _retryHelper); @@ -306,12 +325,12 @@ private void DisplayPublishSummary(ResolvedInput input) // platform flow. A365App* fields on the returned EntraAppSet are placeholder empties. /* var a365 = await factory.CreateProxyAppAsync( - input.ServerName, tenantId, suffix: "A365Proxy", roleDisplay: "A365 Proxy", serviceTreeId: null); + input.ServerName, tenantId, suffix: "A365Proxy", roleDisplay: "A365 Proxy", serviceTreeId: null, ct); if (a365 == null) return null; */ var publicClients = await factory.CreatePublicClientsAppAsync( - input.ServerName, tenantId, serviceTreeId: null, warnings); + input.ServerName, tenantId, serviceTreeId: null, warnings, ct); return new EntraAppSet( A365AppClientId: string.Empty, diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppFactory.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppFactory.cs index 85a370c7..195d8a5a 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppFactory.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppFactory.cs @@ -71,16 +71,18 @@ internal sealed record PublicClientsAppResult(string? ClientId, string? ObjectId } _logger.LogInformation("Created Entra app '{AppName}' (clientId: {ClientId})", appName, app.Value.ClientId); - var secret = await _graphApiService.AddAppPasswordAsync(tenantId, app.Value.ObjectId); + var secret = await _graphApiService.AddAppPasswordAsync(tenantId, app.Value.ObjectId, ct: ct); if (string.IsNullOrWhiteSpace(secret)) { _logger.LogError("Failed to create secret for '{AppName}'. Run with -v for details.", appName); + await TryDeleteOrphanedAppAsync(tenantId, app.Value.ObjectId, appName, "secret-creation failed", ct); return null; } if (string.IsNullOrWhiteSpace(app.Value.ClientId)) { _logger.LogError("{Role} Entra application was created but returned an empty client ID", roleDisplay); + await TryDeleteOrphanedAppAsync(tenantId, app.Value.ObjectId, appName, "empty client ID returned", ct); return null; } @@ -88,6 +90,47 @@ internal sealed record PublicClientsAppResult(string? ClientId, string? ObjectId return new ProxyAppResult(app.Value.ClientId, secret, app.Value.ObjectId, appName); } + /// + /// Best-effort compensating delete for an Entra app that was successfully created but failed + /// a follow-up step (secret creation, post-create validation). Without this, partial failures + /// in leak orphan app registrations into the user's tenant + /// on a likely failure mode (Graph throttling, permission gaps). Delete failures are surfaced + /// as warnings — the caller has already returned null with a clear error, so we don't want a + /// secondary cleanup error to drown the root cause; the user can clean up manually using the + /// objectId we log. + /// + private async Task TryDeleteOrphanedAppAsync(string tenantId, string objectId, string appName, string reason, CancellationToken ct) + { + if (string.IsNullOrWhiteSpace(objectId)) + { + return; + } + + try + { + var deleted = await _graphApiService.DeleteEntraAppAsync(tenantId, objectId, ct); + if (deleted) + { + _logger.LogInformation( + "Rolled back orphan Entra app '{AppName}' (objectId {ObjectId}) after {Reason}.", + appName, objectId, reason); + } + else + { + _logger.LogWarning( + "Could not roll back Entra app '{AppName}' (objectId {ObjectId}) after {Reason}. Delete it manually in the Azure portal.", + appName, objectId, reason); + } + } + catch (Exception ex) + { + _logger.LogWarning( + ex, + "Exception rolling back Entra app '{AppName}' (objectId {ObjectId}) after {Reason}. Delete it manually in the Azure portal.", + appName, objectId, reason); + } + } + /// /// Creates the Public Clients Entra app named {serverName}-PublicClients and sets its /// public-client redirect URIs (broker plugin + canonical localhost/vscode URIs). Best-effort: @@ -124,7 +167,8 @@ internal virtual async Task CreatePublicClientsAppAsync( { var success = await _retryHelper.ExecuteWithRetryAsync( async retryCt => await _graphApiService.UpdateAppPublicClientRedirectUrisAsync(tenantId, objectId, publicClientUris, retryCt), - result => !result); + result => !result, + cancellationToken: ct); if (!success) { var msg = $"Failed to set redirect URIs on Public Clients app '{appName}' after retries."; diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandTests.cs index 278dedfa..b42bc20f 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandTests.cs @@ -115,10 +115,13 @@ public void PublishSubcommand_HasCorrectOptionsWithAliases() var command = DevelopMcpCommand.CreateCommand(_mockLogger, _mockToolingService); var subcommand = command.Subcommands.First(sc => sc.Name == "publish"); - // Assert — description copy was extended in the BYO-parity work to call out the Entra app + - // back-fill orchestration the executor now performs, so match on the Azure-CLI-style verb prefix - // rather than the full string. - subcommand.Description.Should().StartWith("Publish an MCP server to a Dataverse environment"); + // Assert — exact match. The description is intentionally terse (no internal-implementation + // details exposed to end users); a regression that re-adds back-fill / orchestration prose + // should fail this test until the user-facing copy is reviewed. + subcommand.Description.Should().Be( + "Publish an MCP server to a Dataverse environment.", + because: "publish description is user-facing and must stay terse; do not leak internal " + + "orchestration details (Entra apps, back-fill, etc.) into the help output."); var options = subcommand.Options.ToList(); @@ -130,8 +133,14 @@ public void PublishSubcommand_HasCorrectOptionsWithAliases() optionNames.Should().Contain("server-name"); optionNames.Should().Contain("alias"); optionNames.Should().Contain("display-name"); - optionNames.Should().NotContain("tenant-id"); - optionNames.Should().NotContain("service-tree-id"); + optionNames.Should().NotContain( + "tenant-id", + because: "tenant id is auto-detected from the current 'az login' session; exposing " + + "--tenant-id would imply per-publish tenant targeting that the executor does not support."); + optionNames.Should().NotContain( + "service-tree-id", + because: "publish targets a customer's Dataverse env, not a Microsoft corp tenant — " + + "the ServiceTree tagging that --service-tree-id provides is not applicable here."); optionNames.Should().Contain("dry-run"); // Verify critical aliases for Azure CLI compliance diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppFactoryTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppFactoryTests.cs index e64adc35..c8cfba7c 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppFactoryTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppFactoryTests.cs @@ -159,13 +159,20 @@ public async Task CreatePublicClientsAppAsync_HappyPath_ReturnsIdsAndSetsBrokerA capturedUris.Should().ContainSingle(); var uris = capturedUris[0]; - uris.Should().BeEquivalentTo(new[] - { - $"ms-appx-web://Microsoft.AAD.BrokerPlugin/{AppClientId}", - "http://localhost:8080/callback", - "https://vscode.dev/redirect", - "http://localhost", - }, opt => opt.WithStrictOrdering()); + uris.Should().BeEquivalentTo( + new[] + { + $"ms-appx-web://Microsoft.AAD.BrokerPlugin/{AppClientId}", + "http://localhost:8080/callback", + "https://vscode.dev/redirect", + "http://localhost", + }, + opt => opt.WithStrictOrdering(), + because: "Public Clients redirect URIs are part of the OAuth contract with VS Code / " + + "Copilot CLI and the Windows broker. The broker URI is required for WAM/SSO " + + "on Windows, the localhost callbacks support MSAL.NET and Copilot CLI flows, " + + "and vscode.dev/redirect supports the VS Code web client. Silently " + + "adding/removing/reordering entries breaks one of those flows."); } [Fact] From 79319cb5f38f3e4a2f33ecb1e9f9f324299ed939 Mon Sep 17 00:00:00 2001 From: Deepali Garg Date: Mon, 1 Jun 2026 17:41:56 -0700 Subject: [PATCH 09/12] Reinstate secret lifetime support --- .../Commands/RegisterCommandExecutor.cs | 6 ++++-- .../Services/EntraAppFactory.cs | 7 ++++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs index 51a2c33c..75c06961 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs @@ -585,7 +585,8 @@ private void DisplayRegistrationSummary(ResolvedInput input) var factory = new EntraAppFactory(_logger, _graphApiService!, _retryHelper); var a365 = await factory.CreateProxyAppAsync( - input.ServerName, tenantId, suffix: "A365Proxy", roleDisplay: "A365 Proxy", serviceTreeId: input.ServiceTreeId); + input.ServerName, tenantId, suffix: "A365Proxy", roleDisplay: "A365 Proxy", + serviceTreeId: input.ServiceTreeId, lifetimeMonths: input.SecretLifetimeMonths); if (a365 == null) return null; string? remoteProxyClientId = null; @@ -596,7 +597,8 @@ private void DisplayRegistrationSummary(ResolvedInput input) if (input.IsEntra) { var remote = await factory.CreateProxyAppAsync( - input.ServerName, tenantId, suffix: "RemoteProxy", roleDisplay: "Remote Proxy", serviceTreeId: input.ServiceTreeId); + input.ServerName, tenantId, suffix: "RemoteProxy", roleDisplay: "Remote Proxy", + serviceTreeId: input.ServiceTreeId, lifetimeMonths: input.SecretLifetimeMonths); if (remote == null) return null; remoteProxyClientId = remote.ClientId; diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppFactory.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppFactory.cs index 195d8a5a..53a82f96 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppFactory.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppFactory.cs @@ -52,12 +52,17 @@ internal sealed record PublicClientsAppResult(string? ClientId, string? ObjectId /// secret, and validates that the returned client ID is non-empty. Used for both the A365 /// Proxy (publish + register) and the Remote Proxy (register, when auth is EntraOAuth). /// + /// Optional secret lifetime in months, passed through to Graph's + /// addPassword call. When null, Graph applies its default (~2 years). Register surfaces + /// this via --secret-lifetime-months so users in tenants whose appManagementPolicies + /// cap credential lifetimes below 2 years can opt into a shorter expiry; publish leaves it null. internal virtual async Task CreateProxyAppAsync( string serverName, string tenantId, string suffix, string roleDisplay, string? serviceTreeId, + int? lifetimeMonths = null, CancellationToken ct = default) { var appName = $"{serverName}-{suffix}"; @@ -71,7 +76,7 @@ internal sealed record PublicClientsAppResult(string? ClientId, string? ObjectId } _logger.LogInformation("Created Entra app '{AppName}' (clientId: {ClientId})", appName, app.Value.ClientId); - var secret = await _graphApiService.AddAppPasswordAsync(tenantId, app.Value.ObjectId, ct: ct); + var secret = await _graphApiService.AddAppPasswordAsync(tenantId, app.Value.ObjectId, lifetimeMonths: lifetimeMonths, ct: ct); if (string.IsNullOrWhiteSpace(secret)) { _logger.LogError("Failed to create secret for '{AppName}'. Run with -v for details.", appName); From a0da4eca532279dc8eaec818d6118ed952889b52 Mon Sep 17 00:00:00 2001 From: Deepali Garg Date: Mon, 1 Jun 2026 21:54:51 -0700 Subject: [PATCH 10/12] Addressing additional copilot comments --- .../Commands/DevelopMcpCommand.cs | 6 + .../Commands/PublishCommandExecutor.cs | 32 +++- .../DevelopMcpCommandRegressionTests.cs | 148 +++++++++++++++++- .../PublishCommandExecutorDryRunTests.cs | 3 + 4 files changed, 173 insertions(+), 16 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs index 28e89c37..c2d75044 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs @@ -311,6 +311,11 @@ private static Command CreatePublishSubcommand( description: "Publisher name written into the MOS package manifest. Required for custom (user-created) MCP servers; ignored for 1p Microsoft-owned servers (e.g. msdyn_DataverseMCPServer) which always publish as 'Microsoft'."); command.AddOption(publisherNameOption); + var yesOption = new Option( + ["--yes", "-y"], + description: "Skip the interactive 'Proceed with publish? (y/N)' confirmation. Useful for non-interactive contexts (CI/CD pipelines, scripts). Matches az CLI convention."); + command.AddOption(yesOption); + var dryRunOption = new Option("--dry-run", "Show what would be done without executing"); command.AddOption(dryRunOption); @@ -325,6 +330,7 @@ private static Command CreatePublishSubcommand( Alias: context.ParseResult.GetValueForOption(aliasOption), DisplayName: context.ParseResult.GetValueForOption(displayNameOption), PublisherName: context.ParseResult.GetValueForOption(publisherNameOption), + Yes: context.ParseResult.GetValueForOption(yesOption), DryRun: context.ParseResult.GetValueForOption(dryRunOption)); var executor = new PublishCommandExecutor(logger, toolingService, graphApiService); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs index ff92fd5a..59be5b0b 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs @@ -18,6 +18,7 @@ internal record RawPublishArgs( string? Alias, string? DisplayName, string? PublisherName, + bool Yes, bool DryRun); /// @@ -32,6 +33,9 @@ internal record RawPublishArgs( /// internal class PublishCommandExecutor { + // protected (instead of private) on the seam methods + non-sealed class so tests can stub + // out the parts that hit external systems (Azure CLI for tenant detection). The class is + // still internal — overrides only happen in the test assembly via InternalsVisibleTo. private readonly ILogger _logger; private readonly IAgent365ToolingService _toolingService; private readonly GraphApiService? _graphApiService; @@ -62,6 +66,10 @@ private sealed record ResolvedInput // classify ahead of time without knowing the server's mapping, so it leaves the value // null when unspecified and lets the platform decide. public string? PublisherName { get; init; } + + // When true, skip the interactive "Proceed with publish? (y/N)" confirmation. Set via + // --yes / -y. Required for non-interactive contexts (CI scripts, automation). + public required bool Yes { get; init; } } internal sealed record EntraAppSet( @@ -94,14 +102,21 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct return true; } - Console.Write("Proceed with publish? (y/N): "); - var confirmation = Console.ReadLine()?.Trim().ToLowerInvariant(); - if (confirmation != "y" && confirmation != "yes") + if (!input.Yes) { - Console.WriteLine("Publish cancelled."); - // User cancellation is not a failure — exit 0. Matches the register command's same - // prompt-cancel path. - return true; + Console.Write("Proceed with publish? (y/N): "); + var confirmation = Console.ReadLine()?.Trim().ToLowerInvariant(); + if (confirmation != "y" && confirmation != "yes") + { + Console.WriteLine("Publish cancelled."); + // User cancellation is not a failure — exit 0. Matches the register command's same + // prompt-cancel path. + return true; + } + } + else + { + _logger.LogDebug("Skipping interactive confirmation (--yes was supplied)."); } Console.WriteLine(); @@ -276,6 +291,7 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct Alias = alias, DisplayName = displayName, PublisherName = string.IsNullOrWhiteSpace(publisherName) ? null : publisherName, + Yes = args.Yes, DryRun = args.DryRun, }; } @@ -302,7 +318,7 @@ private void DisplayPublishSummary(ResolvedInput input) Console.WriteLine(); } - private async Task DetectTenantIdAsync() + protected virtual async Task DetectTenantIdAsync() { var tenantId = await TenantDetectionHelper.DetectTenantIdAsync(null, _logger); diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandRegressionTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandRegressionTests.cs index d1d74b94..d88d23ea 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandRegressionTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandRegressionTests.cs @@ -95,13 +95,13 @@ public async Task AzureCliStyleParameters_AreAcceptedCorrectly(string command, p } [Fact] - public async Task ServiceIntegration_PublishCommand_AcceptsAllNamedParameters() + public async Task PublishCommand_AcceptsAllNamedParameters_InDryRun() { - // Verifies the publish CLI parses every documented flag without error. The publish flow now - // orchestrates Entra app creation + redirect-URI back-fill via GraphApiService (mirroring - // register-external-mcp-server), so end-to-end "params flow to PublishServerAsync" can't be - // exercised here without mocking Graph too — that path is covered by the - // regression test and by manual E2E testing. + // Verifies the publish CLI parses every documented flag without error. Dry-run + // short-circuits before any platform call, so this is a pure CLI parsing test — + // it does NOT verify that the parsed values flow into PublishServerAsync. + // That contract is covered by PublishCommand_ForwardsParsedParametersToToolingService + // (which mocks Graph + tenant detection so the non-dry-run path can be exercised). // Arrange var testEnvId = "test-environment-123"; @@ -121,8 +121,140 @@ public async Task ServiceIntegration_PublishCommand_AcceptsAllNamedParameters() }); // Assert — successful parse + dispatch, no service calls. - result.Should().Be(0); - await _mockToolingService.DidNotReceive().PublishServerAsync(Arg.Any(), Arg.Any(), Arg.Any()); + result.Should().Be( + 0, + because: "dry-run should never trigger a non-zero exit code when all flags parse cleanly."); + await _mockToolingService.DidNotReceive().PublishServerAsync( + Arg.Any(), Arg.Any(), Arg.Any()); + } + + /// + /// Strengthened contract test: verifies that every parsed publish flag flows into the actual + /// call. Exercises the non-dry-run + /// path by (a) using --yes to bypass the interactive confirmation prompt, (b) mocking + /// so Entra app creation succeeds without a real tenant, and + /// (c) subclassing to stub out tenant auto-detection so + /// the executor doesn't shell out to Azure CLI in CI. Catches breakage of the CLI-to-service + /// contract (alias/display-name/publisher-name mapping, request shaping, and selecting the + /// correct service method) that the dry-run-only test above can't catch. + /// + [Fact] + public async Task PublishCommand_ForwardsParsedParametersToToolingService() + { + // Arrange + const string TestTenantId = "test-tenant-99999"; + const string TestEnvironmentId = "test-env-forward"; + const string TestServerName = "msdyn_TestServer"; + const string TestAlias = "test-alias-forward"; + const string TestDisplayName = "Test Display Forward"; + const string TestPublisherName = "Contoso Forward"; + const string TestPublicClientsObjectId = "public-clients-object-id"; + const string TestPublicClientsClientId = "public-clients-client-id"; + + var logger = Substitute.For(); + var toolingService = Substitute.For(); + var graphApiService = Substitute.For(); + + // Mock Graph so CreateEntraAppsAsync → factory.CreatePublicClientsAppAsync succeeds. + graphApiService.CreateEntraAppAsync( + TestTenantId, Arg.Any(), serviceTreeId: Arg.Any(), Arg.Any()) + .Returns(Task.FromResult<(string ObjectId, string ClientId)?>( + (TestPublicClientsObjectId, TestPublicClientsClientId))); + graphApiService.UpdateAppPublicClientRedirectUrisAsync( + TestTenantId, TestPublicClientsObjectId, Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(true)); + + // Mock Graph for ConfigureEntraAppsAsync → required-resource-access grant on Public Clients. + graphApiService.GetOAuth2PermissionScopeIdAsync( + TestTenantId, Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(Guid.NewGuid())); + graphApiService.AddRequiredResourceAccessAsync( + TestTenantId, Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(true)); + + // Capture what gets forwarded to PublishServerAsync. + string? capturedEnvId = null; + string? capturedServerName = null; + PublishMcpServerRequest? capturedRequest = null; + toolingService.PublishServerAsync( + Arg.Do(e => capturedEnvId = e), + Arg.Do(s => capturedServerName = s), + Arg.Do(r => capturedRequest = r), + Arg.Any()) + .Returns(new PublishMcpServerResponse + { + Status = "Success", + McpServerAppId = Guid.NewGuid().ToString(), + McpServerScope = "Tools.ListInvoke.All", + }); + + var executor = new TestablePublishCommandExecutor( + logger, toolingService, graphApiService, TestTenantId); + + var args = new RawPublishArgs( + EnvironmentId: TestEnvironmentId, + ServerName: TestServerName, + Alias: TestAlias, + DisplayName: TestDisplayName, + PublisherName: TestPublisherName, + Yes: true, + DryRun: false); + + // Act + var result = await executor.ExecuteAsync(args); + + // Assert + result.Should().BeTrue( + because: "the happy path with all dependencies mocked must return true so the publish " + + "handler exits 0."); + capturedRequest.Should().NotBeNull( + because: "PublishServerAsync must be invoked once the prompt is skipped, the tenant " + + "is detected, and the Public Clients app is created."); + capturedEnvId.Should().Be( + TestEnvironmentId, + because: "--environment-id must flow unchanged into PublishServerAsync's first positional arg."); + capturedServerName.Should().Be( + TestServerName, + because: "--server-name must flow unchanged into PublishServerAsync's second positional arg."); + capturedRequest!.Alias.Should().Be( + TestAlias, + because: "--alias must populate request.Alias; this is the platform's `name` for the published row."); + capturedRequest.DisplayName.Should().Be( + TestDisplayName, + because: "--display-name must populate request.DisplayName; the platform's v2 validator " + + "requires it and surfaces it in MOS."); + capturedRequest.PublisherName.Should().Be( + TestPublisherName, + because: "--publisher-name must populate request.PublisherName so the MOS manifest's " + + "developer field is set for custom servers (the platform rejects empty values " + + "for non-1p servers)."); + capturedRequest.PublicClientsAppId.Should().Be( + TestPublicClientsClientId, + because: "the just-created Public Clients Entra app's clientId must be carried to the " + + "platform so it can be echoed back and the CLI can grant the PPMI scope on it " + + "post-publish."); + } + + /// + /// Test-only subclass of that stubs out + /// with a known value, so the + /// strengthened contract test doesn't need to shell out to az account show in CI. + /// + private sealed class TestablePublishCommandExecutor : PublishCommandExecutor + { + private readonly string? _tenantId; + + public TestablePublishCommandExecutor( + ILogger logger, + IAgent365ToolingService toolingService, + GraphApiService? graphApiService, + string? tenantId) + : base(logger, toolingService, graphApiService) + { + _tenantId = tenantId; + } + + protected override Task DetectTenantIdAsync() => Task.FromResult(_tenantId); } [Fact] diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorDryRunTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorDryRunTests.cs index 87181b02..5dbaebf7 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorDryRunTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorDryRunTests.cs @@ -37,6 +37,7 @@ public async Task ExecuteAsync_DryRun_LogsEntraAppNamesDerivedFromServerName() Alias: alias, DisplayName: "Test Display", PublisherName: null, + Yes: false, DryRun: true); var executor = new PublishCommandExecutor(logger, toolingService, graphApiService: null); @@ -89,6 +90,7 @@ public async Task ExecuteAsync_DryRun_OmitsA365ProxyApp_WhileCustomConnectorFlow Alias: alias, DisplayName: "Test Display", PublisherName: null, + Yes: false, DryRun: true); var executor = new PublishCommandExecutor(logger, toolingService, graphApiService: null); @@ -139,6 +141,7 @@ public async Task ExecuteAsync_DryRun_AcceptsPublisherName_WithoutCallingPlatfor Alias: "myAlias", DisplayName: "Test Display", PublisherName: "Contoso", + Yes: false, DryRun: true); var executor = new PublishCommandExecutor(logger, toolingService, graphApiService: null); From 62c9e70233dcee3554f84f1b7b158d5e9cb29a58 Mon Sep 17 00:00:00 2001 From: Deepali Garg Date: Wed, 3 Jun 2026 12:44:24 -0700 Subject: [PATCH 11/12] Removing a365 prxy code and other comments --- CHANGELOG.md | 5 + .../Commands/DevelopMcpCommand.cs | 5 +- .../Commands/PublishCommandExecutor.cs | 135 ++---------------- .../Commands/RegisterCommandExecutor.cs | 28 ++-- .../Models/PublishMcpServerRequest.cs | 15 -- .../Models/PublishMcpServerResponse.cs | 16 +-- ...raAppFactory.cs => EntraAppProvisioner.cs} | 6 +- .../PublishCommandExecutorDryRunTests.cs | 68 ++------- ...ryTests.cs => EntraAppProvisionerTests.cs} | 34 ++--- 9 files changed, 60 insertions(+), 252 deletions(-) rename src/Microsoft.Agents.A365.DevTools.Cli/Services/{EntraAppFactory.cs => EntraAppProvisioner.cs} (97%) rename src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/{EntraAppFactoryTests.cs => EntraAppProvisionerTests.cs} (84%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 116c145a..5dca78c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,8 @@ Agents provisioned before this release need `Agent365.Observability.OtelWrite` g - `setup requirements` now detects and auto-repairs the **`wids` optional claim** on the CLI client app's access tokens. Without this claim, the CLI cannot read the signed-in user's directory roles from the token and silently skips the tenant-wide consent step on the agent blueprint — the user-visible symptom is a blueprint with `inheritablePermissions.kind=allAllowed` but no permissions actually granted on the blueprint service principal. When the running user has admin authority over the app registration, the CLI patches `optionalClaims.accessToken` to include `wids` and clears the local MSAL token cache so the next acquisition carries the new claim. The well-known CLI app is also now created with `wids` already configured. Non-admin users get explicit Azure portal click-path remediation. - `setup requirements` now auto-provisions the `Application.Read.All` consented permission when it is declared on the CLI client app but missing from the tenant's OAuth2 consent grant. Required by the wids check (which queries `/v1.0/applications`) and the existing redirect-URI / public-client validators. - `--secret-lifetime-months ` option (and matching `secretLifetimeMonths` field in the `--input-file` JSON) on `develop-mcp register-external-mcp-server` — controls the lifetime of the client secrets created on the A365Proxy and RemoteProxy Entra apps. Valid range `1-24`; omit to use the Graph default (~2 years). Calendar-aware (uses `DateTimeOffset.AddMonths`, so Jan 31 + 1 month → Feb 28/29). Added so tenants with an `appManagementPolicies` cap on client-secret lifetime — previously a hard failure inside `CreateEntraAppsAsync` with a generic "Failed to create secret" message — can fit registration inside their tenant's policy. When Graph rejects the requested (or default) lifetime with a tenant-policy error, the CLI now emits an actionable error naming the flag and the attempted value (e.g. `Tenant Entra ID policy rejected the requested 12-month lifetime ... Pass --secret-lifetime-months N with a smaller value (e.g. --secret-lifetime-months 3) that fits inside your tenant's appManagementPolicies cap.`) instead of the previous generic failure. +- `--publisher-name` / `-p` option on `develop-mcp publish` — sets the publisher name written into the published MCP server's package metadata. Required for custom (user-created) MCP servers; ignored for 1p Microsoft-owned servers (e.g. `msdyn_DataverseMCPServer`), which always publish as "Microsoft". Prompted interactively when omitted. +- `--yes` / `-y` option on `develop-mcp publish` — skips the interactive "Proceed with publish? (y/N)" confirmation. ### Fixed - `setup all` now exits silently on Ctrl+C instead of printing `ERROR: Setup failed: A task was canceled.` followed by a misleading partial summary. @@ -112,6 +114,9 @@ Agents provisioned before this release need `Agent365.Observability.OtelWrite` g - **`a365 config display` removed** — use `a365 query-entra blueprint-scopes` to inspect live blueprint permissions and consent state. - **`a365 config permissions` removed** — replace with `a365 setup permissions custom --resource-app-id --scopes `. - **`--config`/`-c` option removed from all commands** — config file is now always resolved from the current directory (`a365.config.json`). Scripts passing `--config ` will receive a parse error; change directory before running the CLI instead. +- **`--tenant-id` / `-t` removed from `a365 develop-mcp register-external-mcp-server`** — the tenant is now auto-detected from the current `az login` session. Scripts passing `-t ` / `--tenant-id ` will receive a System.CommandLine parse error; run `az login --tenant ` (or `az account set --subscription `) to target a specific tenant instead. +- **`a365 develop-mcp publish` now creates a `-PublicClients` Entra app registration in your tenant** — the publish orchestration runs CLI-side, so after each publish you will see a new app registration named `-PublicClients` in your tenant's Entra ID. These are created by the CLI; clean them up with the same name if you unpublish. +- **`a365 develop-mcp publish` now requires the `Application.ReadWrite.All` Microsoft Graph permission** — needed to create the Entra app registration above. Running publish with only read-only Graph permissions will fail. Grant `Application.ReadWrite.All` to the account (or app) running the CLI before publishing. - **`--agent-instance-only` renamed to `--agent-registration-only`** on `a365 setup all` — update any scripts using the old flag name. - **`setup permissions custom --resource-app-id --scopes` applies permissions directly to Entra ID** — unlike the former `a365 config permissions` which only wrote to `a365.config.json`, this inline mode immediately mutates the live blueprint in Entra and cannot be undone by editing a config file. - `a365 setup` now writes the `Agent365Observability` placeholder section (`AgentId`, `AgentBlueprintId`, `TenantId`, `AgentName`, `AgentDescription`) and `EnableAgent365Exporter: false` to `appsettings.json` (.NET) and `ENABLE_A365_OBSERVABILITY_EXPORTER=false` to `.env` (Python/Node.js), so observability configuration is pre-populated for all three platforms after running setup diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs index c2d75044..1f05a65f 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs @@ -288,6 +288,7 @@ private static Command CreatePublishSubcommand( var envIdOption = new Option( ["--environment-id", "-e"], description: "Dataverse environment ID"); + envIdOption.IsRequired = false; // Allow null so we can prompt command.AddOption(envIdOption); var serverNameOption = new Option( @@ -308,12 +309,12 @@ private static Command CreatePublishSubcommand( var publisherNameOption = new Option( ["--publisher-name", "-p"], - description: "Publisher name written into the MOS package manifest. Required for custom (user-created) MCP servers; ignored for 1p Microsoft-owned servers (e.g. msdyn_DataverseMCPServer) which always publish as 'Microsoft'."); + description: "Publisher name for the MCP Server. Required for custom (user-created) MCP servers; ignored for 1p Microsoft-owned servers (e.g. msdyn_DataverseMCPServer) which always publish as 'Microsoft'."); command.AddOption(publisherNameOption); var yesOption = new Option( ["--yes", "-y"], - description: "Skip the interactive 'Proceed with publish? (y/N)' confirmation. Useful for non-interactive contexts (CI/CD pipelines, scripts). Matches az CLI convention."); + description: "Skip the interactive 'Proceed with publish? (y/N)' confirmation."); command.AddOption(yesOption); var dryRunOption = new Option("--dry-run", "Show what would be done without executing"); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs index 59be5b0b..aa77c1f5 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs @@ -26,10 +26,6 @@ internal record RawPublishArgs( /// for BYO register: the CLI creates the Public Clients /// Entra app in the user's own tenant, calls the platform publish endpoint, and back-fills the /// PPMI scope grant on it after the platform resolves the underlying server's PPMI identity. -/// NOTE: A365 Proxy Entra app creation + redirect-URI back-fill are TEMPORARILY DISABLED while -/// the platform-side custom connector flow is commented out (see PublishMCPServerV2Async in -/// MCPDataverseEnvironmentService). Reinstate the proxy-related blocks below together with the -/// platform-side flow. /// internal class PublishCommandExecutor { @@ -73,10 +69,6 @@ private sealed record ResolvedInput } internal sealed record EntraAppSet( - string A365AppClientId, - string A365AppSecret, - string A365AppObjectId, - string A365AppName, string? PublicClientsClientId, string? PublicClientsObjectId, string PublicClientsAppName); @@ -90,13 +82,6 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct if (input.DryRun) { - // TEMPORARILY DISABLED: A365 Proxy dry-run lines (proxy app creation + redirect URI - // back-fill). Reinstate together with the corresponding logic in CreateEntraAppsAsync - // and ConfigureEntraAppsAsync. - /* - _logger.LogInformation("[DRY RUN] Would create Entra apps '{A365}' and '{PublicClients}' in tenant", $"{input.ServerName}-A365Proxy", $"{input.ServerName}-PublicClients"); - _logger.LogInformation("[DRY RUN] Would call publish endpoint and back-fill redirect URI + PPMI scope on the created apps"); - */ _logger.LogInformation("[DRY RUN] Would create Entra app '{PublicClients}' in tenant", $"{input.ServerName}-PublicClients"); _logger.LogInformation("[DRY RUN] Would call publish endpoint and back-fill PPMI scope on the created app"); return true; @@ -138,26 +123,10 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct ct.ThrowIfCancellationRequested(); - // TEMPORARILY DISABLED: A365 Proxy client-id parse + creds assignment to the publish request. - // The platform's v2 publish ignores these fields while the custom connector flow is - // disabled (see PublishMCPServerV2Async Step 3 comment block in MCPDataverseEnvironmentService). - // Reinstate the parse block, the apps == null short-circuit above, and the two request - // fields together when the proxy flow returns. - /* - if (!Guid.TryParse(apps.A365AppClientId, out var a365ProxyClientId)) - { - _logger.LogError("A365 Proxy Entra app returned an invalid client ID '{ClientId}'. Expected a GUID. Cannot continue publish.", apps.A365AppClientId); - await RollbackEntraAppsAsync(apps, tenantId, ct); - return false; - } - */ - var request = new PublishMcpServerRequest { Alias = input.Alias, DisplayName = input.DisplayName, - // A365ProxyClientId = a365ProxyClientId, - // A365ProxyClientSecret = apps.A365AppSecret, PublicClientsAppId = apps.PublicClientsClientId, PublisherName = input.PublisherName, }; @@ -166,10 +135,8 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct try { // Hits the platform's v2 publish endpoint via the tooling service, which performs the - // full elevation orchestration (PPMI provisioning, MOS upload). A365 Proxy CMS - // connector creation is TEMPORARILY DISABLED on the platform side (see - // PublishMCPServerV2Async Step 3 comment block) while the custom connector flow is - // being re-evaluated. The platform's v1 endpoint remains for older CLI binaries. + // full elevation orchestration (PPMI provisioning, MOS upload). The platform's v1 + // endpoint remains for older CLI binaries. publishResponse = await _toolingService.PublishServerAsync(input.EnvironmentId, input.ServerName, request, ct); } catch (Exception ex) @@ -333,26 +300,12 @@ private void DisplayPublishSummary(ResolvedInput input) private async Task CreateEntraAppsAsync(ResolvedInput input, string tenantId, List warnings, CancellationToken ct = default) { - var factory = new EntraAppFactory(_logger, _graphApiService!, _retryHelper); - - // TEMPORARILY DISABLED: A365 Proxy Entra app creation. The platform-side custom connector - // flow that consumed these credentials is commented out, so creating the app here would - // leak an unused Entra registration in the user's tenant. Reinstate together with the - // platform flow. A365App* fields on the returned EntraAppSet are placeholder empties. - /* - var a365 = await factory.CreateProxyAppAsync( - input.ServerName, tenantId, suffix: "A365Proxy", roleDisplay: "A365 Proxy", serviceTreeId: null, ct); - if (a365 == null) return null; - */ - - var publicClients = await factory.CreatePublicClientsAppAsync( + var provisioner = new EntraAppProvisioner(_logger, _graphApiService!, _retryHelper); + + var publicClients = await provisioner.CreatePublicClientsAppAsync( input.ServerName, tenantId, serviceTreeId: null, warnings, ct); return new EntraAppSet( - A365AppClientId: string.Empty, - A365AppSecret: string.Empty, - A365AppObjectId: string.Empty, - A365AppName: string.Empty, PublicClientsClientId: publicClients.ClientId, PublicClientsObjectId: publicClients.ObjectId, PublicClientsAppName: publicClients.AppName); @@ -366,18 +319,12 @@ internal async Task RollbackEntraAppsAsync(EntraAppSet apps, string tenantId, Ca { if (_graphApiService is null) { - _logger.LogWarning("Graph API service is unavailable; cannot roll back Entra apps '{A365}' / '{PublicClients}'. Delete them manually in the Azure portal.", apps.A365AppName, apps.PublicClientsAppName); + _logger.LogWarning("Graph API service is unavailable; cannot roll back Entra app '{PublicClients}'. Delete it manually in the Azure portal.", apps.PublicClientsAppName); return; } _logger.LogInformation("Rolling back Entra app registrations created for failed publish..."); - // TEMPORARILY DISABLED: A365 Proxy delete. The proxy Entra app is no longer created - // (see CreateEntraAppsAsync). Reinstate together with the proxy app creation. - /* - await DeleteOneAsync(apps.A365AppObjectId, apps.A365AppClientId, apps.A365AppName, ct); - */ - if (!string.IsNullOrWhiteSpace(apps.PublicClientsObjectId)) { await DeleteOneAsync(apps.PublicClientsObjectId, apps.PublicClientsClientId, apps.PublicClientsAppName, ct); @@ -420,30 +367,11 @@ private async Task ConfigureEntraAppsAsync( var tasks = new List(); var concurrentWarnings = new System.Collections.Concurrent.ConcurrentBag(); - // TEMPORARILY DISABLED: A365 Proxy redirect-URI back-fill. The platform's v2 publish no - // longer creates the proxy connector, so A365ProxyRedirectUri always comes back null and - // there is no A365 Proxy Entra app to write a redirect URI onto. Reinstate together with - // the proxy app creation in CreateEntraAppsAsync. - /* - var a365RedirectUri = response.A365ProxyRedirectUri; - - if (!string.IsNullOrWhiteSpace(a365RedirectUri)) - { - tasks.Add(UpdateA365RedirectUrisAsync(tenantId, apps, a365RedirectUri, concurrentWarnings, ct)); - } - else - { - var msg = "A365 Proxy redirect URI was not returned by the server. Redirect URI configuration skipped."; - _logger.LogWarning(msg); - concurrentWarnings.Add(msg); - } - */ - - // Grant required-resource-access on the just-created A365 Proxy + Public Clients Entra apps. + // Grant required-resource-access on the just-created Public Clients Entra app. // The platform resolves the right resource per server type (Custom: managedidentityid; app-based // / Dataverse MCP: 1p mappings; fallback: platform's own app id) and returns both the resource // app id and the scope name. We look up the scope guid on the resource app, then add it as - // requiredResourceAccess on each of the two Entra apps we created. + // requiredResourceAccess on the Entra app we created. var resourceAppId = response.McpServerAppId; var resourceScopeName = response.McpServerScope; Guid? resourceScopeId = null; @@ -474,13 +402,6 @@ private async Task ConfigureEntraAppsAsync( if (resourceScopeId.HasValue) { - // TEMPORARILY DISABLED: required-resource-access grant on the A365 Proxy app. The proxy - // app is no longer created (see CreateEntraAppsAsync). Reinstate together with the - // proxy app creation. - /* - tasks.Add(AddRequiredResourceAccessAsync(tenantId, apps.A365AppObjectId, apps.A365AppName, resourceAppId!, resourceScopeId.Value, concurrentWarnings, ct)); - */ - if (apps.PublicClientsObjectId != null) { tasks.Add(AddRequiredResourceAccessAsync(tenantId, apps.PublicClientsObjectId, apps.PublicClientsAppName, resourceAppId!, resourceScopeId.Value, concurrentWarnings, ct)); @@ -499,46 +420,6 @@ private async Task ConfigureEntraAppsAsync( warnings.Add(w); } - // TEMPORARILY DISABLED: UpdateA365RedirectUrisAsync. Sole caller in ConfigureEntraAppsAsync is - // commented out. Reinstate together with the A365 Proxy app creation in CreateEntraAppsAsync. - /* - private async Task UpdateA365RedirectUrisAsync( - string tenantId, - EntraAppSet apps, - string a365RedirectUri, - System.Collections.Concurrent.ConcurrentBag concurrentWarnings, - CancellationToken ct = default) - { - try - { - var a365TcUri = DevelopMcpCommand.AddTcPrefix(a365RedirectUri); - var a365NonTcUri = DevelopMcpCommand.RemoveTcPrefix(a365RedirectUri); - var a365Uris = DevelopMcpCommand.BuildRedirectUriList(a365RedirectUri, a365TcUri, a365NonTcUri); - _logger.LogDebug("Updating redirect URIs on '{AppName}' ({ObjectId})", apps.A365AppName, apps.A365AppObjectId); - var success = await _retryHelper.ExecuteWithRetryAsync( - async retryCt => await _graphApiService!.UpdateAppRedirectUrisAsync(tenantId, apps.A365AppObjectId, a365Uris, retryCt), - result => !result, - cancellationToken: ct); - if (!success) - { - var msg = $"Failed to update redirect URIs on A365 Proxy app '{apps.A365AppName}' after retries."; - _logger.LogError(msg); - concurrentWarnings.Add(msg); - } - else - { - _logger.LogInformation("Updated redirect URIs on '{AppName}'", apps.A365AppName); - } - } - catch (Exception ex) - { - var msg = $"Failed to update redirect URIs on A365 Proxy app: {ex.Message}"; - _logger.LogError(msg); - concurrentWarnings.Add(msg); - } - } - */ - private async Task AddRequiredResourceAccessAsync( string tenantId, string appObjectId, diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs index 75c06961..622e23c0 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs @@ -582,12 +582,12 @@ private void DisplayRegistrationSummary(ResolvedInput input) private async Task CreateEntraAppsAsync( ResolvedInput input, string tenantId, List warnings) { - var factory = new EntraAppFactory(_logger, _graphApiService!, _retryHelper); + var provisioner = new EntraAppProvisioner(_logger, _graphApiService!, _retryHelper); - var a365 = await factory.CreateProxyAppAsync( + var a365ProxyApp = await provisioner.CreateProxyAppAsync( input.ServerName, tenantId, suffix: "A365Proxy", roleDisplay: "A365 Proxy", serviceTreeId: input.ServiceTreeId, lifetimeMonths: input.SecretLifetimeMonths); - if (a365 == null) return null; + if (a365ProxyApp == null) return null; string? remoteProxyClientId = null; string? remoteProxySecret = null; @@ -596,25 +596,25 @@ private void DisplayRegistrationSummary(ResolvedInput input) if (input.IsEntra) { - var remote = await factory.CreateProxyAppAsync( + var remoteProxyApp = await provisioner.CreateProxyAppAsync( input.ServerName, tenantId, suffix: "RemoteProxy", roleDisplay: "Remote Proxy", serviceTreeId: input.ServiceTreeId, lifetimeMonths: input.SecretLifetimeMonths); - if (remote == null) return null; + if (remoteProxyApp == null) return null; - remoteProxyClientId = remote.ClientId; - remoteProxySecret = remote.Secret; - remoteProxyObjectId = remote.ObjectId; - remoteProxyAppName = remote.AppName; + remoteProxyClientId = remoteProxyApp.ClientId; + remoteProxySecret = remoteProxyApp.Secret; + remoteProxyObjectId = remoteProxyApp.ObjectId; + remoteProxyAppName = remoteProxyApp.AppName; } - var publicClients = await factory.CreatePublicClientsAppAsync( + var publicClients = await provisioner.CreatePublicClientsAppAsync( input.ServerName, tenantId, serviceTreeId: input.ServiceTreeId, warnings); return new EntraAppSet( - A365AppClientId: a365.ClientId, - A365AppSecret: a365.Secret, - A365AppObjectId: a365.ObjectId, - A365AppName: a365.AppName, + A365AppClientId: a365ProxyApp.ClientId, + A365AppSecret: a365ProxyApp.Secret, + A365AppObjectId: a365ProxyApp.ObjectId, + A365AppName: a365ProxyApp.AppName, RemoteProxyClientId: remoteProxyClientId, RemoteProxySecret: remoteProxySecret, RemoteProxyObjectId: remoteProxyObjectId, diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerRequest.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerRequest.cs index 96924387..ceb92080 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerRequest.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerRequest.cs @@ -1,6 +1,5 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -using System; using System.Text.Json.Serialization; namespace Microsoft.Agents.A365.DevTools.Cli.Models; @@ -22,20 +21,6 @@ public class PublishMcpServerRequest [JsonPropertyName("DisplayName")] public required string DisplayName { get; set; } - /// - /// A365 Proxy Entra app client id created CLI-side at publish time. Paired with - /// . When provided, the platform creates an A365 Proxy CMS - /// connector keyed by server name so the published server is reachable through Power Platform / Copilot. - /// - [JsonPropertyName("a365ProxyClientId")] - public Guid? A365ProxyClientId { get; set; } - - /// - /// A365 Proxy Entra app client secret. Paired with . - /// - [JsonPropertyName("a365ProxyClientSecret")] - public string? A365ProxyClientSecret { get; set; } - /// /// Public Clients (VS Code / Copilot CLI) Entra app client id created CLI-side. Carried in the /// request so the platform echoes it back in the response and the CLI can wire the PPMI diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerResponse.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerResponse.cs index 999bae6b..2ea1e2b9 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerResponse.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerResponse.cs @@ -27,7 +27,7 @@ public class PublishMcpServerResponse /// server type — Custom servers from managedidentityid on the mcpserver row, app-based /// / Dataverse MCP servers from 1p server-to-app mappings, fallback to the platform's own app id. /// The CLI uses this together with to look up the scope id and grant - /// required-resource-access on the A365 Proxy + Public Clients Entra apps. + /// required-resource-access on the Public Clients Entra app. /// [JsonPropertyName("McpServerAppId")] public string? McpServerAppId { get; set; } @@ -40,20 +40,6 @@ public class PublishMcpServerResponse [JsonPropertyName("McpServerScope")] public string? McpServerScope { get; set; } - /// - /// CMS connector id created at publish time for the A365 Proxy connector, or null when the CLI - /// didn't pass Entra app credentials (older CLI flow) and no connector was created. - /// - [JsonPropertyName("A365ProxyConnectorId")] - public string? A365ProxyConnectorId { get; set; } - - /// - /// OAuth redirect URI for the A365 Proxy connector. The CLI writes this onto the just-created - /// A365 Proxy Entra app's redirect URI list (with tc / non-tc variants) so OAuth flows complete. - /// - [JsonPropertyName("A365ProxyRedirectUri")] - public string? A365ProxyRedirectUri { get; set; } - /// /// Public Clients Entra app client id, echoed back from the request so post-response /// orchestration can grant the PPMI scope onto it. diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppFactory.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs similarity index 97% rename from src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppFactory.cs rename to src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs index 53a82f96..a46a36c1 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppFactory.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs @@ -10,10 +10,10 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Services; /// Creates the Entra app registrations needed by publish and register flows. Both flows produce /// the same two app shapes (a confidential proxy app with a secret, and a public-client app with /// canonical redirect URIs); register additionally produces a second proxy app for the remote -/// MCP server. This factory centralizes the creation steps and their error-handling so the +/// MCP server. This provisioner centralizes the creation steps and their error-handling so the /// command executors can compose them without duplicating the per-step null checks and logging. /// -internal class EntraAppFactory +internal class EntraAppProvisioner { private static readonly string[] PublicClientCanonicalRedirectUris = [ @@ -26,7 +26,7 @@ internal class EntraAppFactory private readonly GraphApiService _graphApiService; private readonly RetryHelper _retryHelper; - internal EntraAppFactory(ILogger logger, GraphApiService graphApiService, RetryHelper retryHelper) + internal EntraAppProvisioner(ILogger logger, GraphApiService graphApiService, RetryHelper retryHelper) { _logger = logger; _graphApiService = graphApiService; diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorDryRunTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorDryRunTests.cs index 5dbaebf7..1989ea12 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorDryRunTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorDryRunTests.cs @@ -13,75 +13,25 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Commands; /// /// Tests for dry-run output. The dry-run log must mirror the /// real Entra app naming scheme (derived from ServerName) so users can predict what will be -/// created. Earlier the log used Alias, which diverged from CreateEntraAppsAsync's -/// {ServerName}-A365Proxy / {ServerName}-PublicClients naming. +/// created — the {ServerName}-PublicClients app. /// public class PublishCommandExecutorDryRunTests { - [Fact(Skip = "A365 Proxy Entra app creation is TEMPORARILY DISABLED in PublishCommandExecutor while the platform custom connector flow is commented out. Dry-run log no longer mentions the A365Proxy app or the redirect-URI back-fill. Re-enable together with CreateEntraAppsAsync's proxy creation.")] - public async Task ExecuteAsync_DryRun_LogsEntraAppNamesDerivedFromServerName() - { - var logger = Substitute.For(); - var toolingService = Substitute.For(); - - // ServerName and Alias are intentionally distinct so a regression that reverts to Alias - // would surface in the negative assertion below. - const string serverName = "mcp_TestServer"; - const string alias = "myAlias"; - const string expectedA365AppName = "mcp_TestServer-A365Proxy"; - const string expectedPublicClientsAppName = "mcp_TestServer-PublicClients"; - - var args = new RawPublishArgs( - EnvironmentId: "00000000-0000-0000-0000-000000000000", - ServerName: serverName, - Alias: alias, - DisplayName: "Test Display", - PublisherName: null, - Yes: false, - DryRun: true); - - var executor = new PublishCommandExecutor(logger, toolingService, graphApiService: null); - - await executor.ExecuteAsync(args, CancellationToken.None); - - logger.Received(1).Log( - LogLevel.Information, - Arg.Any(), - Arg.Is(o => - o.ToString()!.Contains("[DRY RUN] Would create Entra apps") && - o.ToString()!.Contains(expectedA365AppName) && - o.ToString()!.Contains(expectedPublicClientsAppName) && - !o.ToString()!.Contains($"{alias}-A365Proxy") && - !o.ToString()!.Contains($"{alias}-PublicClients")), - Arg.Any(), - Arg.Any>()); - - logger.Received(1).Log( - LogLevel.Information, - Arg.Any(), - Arg.Is(o => o.ToString()!.Contains("Would call publish endpoint and back-fill")), - Arg.Any(), - Arg.Any>()); - - await toolingService.DidNotReceiveWithAnyArgs().PublishServerAsync(default!, default!, default!, default); - } - /// - /// While the A365 Proxy flow is disabled, the dry-run log must (a) mention only the Public - /// Clients app — not the A365 Proxy app — derived from ServerName, (b) replace the - /// redirect-URI back-fill line with a PPMI-scope-only back-fill line, and (c) still skip the - /// platform publish call. Acts as the canary: if the proxy line silently comes back without - /// the corresponding logic being re-enabled, this fails. + /// The dry-run log must (a) name only the Public Clients app — derived from ServerName, + /// not Alias — (b) describe a PPMI-scope-only back-fill (no redirect-URI back-fill), and + /// (c) skip the platform publish call entirely. /// [Fact] - public async Task ExecuteAsync_DryRun_OmitsA365ProxyApp_WhileCustomConnectorFlowDisabled() + public async Task ExecuteAsync_DryRun_NamesPublicClientsApp_AndBackfillsPpmiScopeOnly() { var logger = Substitute.For(); var toolingService = Substitute.For(); + // ServerName and Alias are intentionally distinct so a regression that reverts to Alias + // would surface in the negative assertion below. const string serverName = "mcp_TestServer"; const string alias = "myAlias"; - const string disabledA365AppName = "mcp_TestServer-A365Proxy"; const string expectedPublicClientsAppName = "mcp_TestServer-PublicClients"; var args = new RawPublishArgs( @@ -97,14 +47,14 @@ public async Task ExecuteAsync_DryRun_OmitsA365ProxyApp_WhileCustomConnectorFlow await executor.ExecuteAsync(args, CancellationToken.None); - // The active dry-run line names only Public Clients and does NOT name A365 Proxy. + // The dry-run line names the Public Clients app (derived from ServerName, not Alias). logger.Received(1).Log( LogLevel.Information, Arg.Any(), Arg.Is(o => o.ToString()!.Contains("[DRY RUN] Would create Entra app") && o.ToString()!.Contains(expectedPublicClientsAppName) && - !o.ToString()!.Contains(disabledA365AppName)), + !o.ToString()!.Contains($"{alias}-PublicClients")), Arg.Any(), Arg.Any>()); diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppFactoryTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs similarity index 84% rename from src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppFactoryTests.cs rename to src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs index c8cfba7c..4b7c6624 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppFactoryTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/EntraAppProvisionerTests.cs @@ -11,12 +11,12 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Services; /// -/// Tests for . The factory absorbed the per-app creation flows that -/// previously lived inline in PublishCommandExecutor and RegisterCommandExecutor. These tests -/// pin every branch (success + each failure mode) so the executors can compose the factory +/// Tests for . The provisioner absorbed the per-app creation flows +/// that previously lived inline in PublishCommandExecutor and RegisterCommandExecutor. These tests +/// pin every branch (success + each failure mode) so the executors can compose the provisioner /// without needing their own per-step coverage. /// -public class EntraAppFactoryTests +public class EntraAppProvisionerTests { private const string ServerName = "mcp_TestServer"; private const string TenantId = "00000000-0000-0000-0000-0000000000aa"; @@ -27,14 +27,14 @@ public class EntraAppFactoryTests private readonly ILogger _logger; private readonly GraphApiService _graph; private readonly RetryHelper _retryHelper; - private readonly EntraAppFactory _factory; + private readonly EntraAppProvisioner _provisioner; - public EntraAppFactoryTests() + public EntraAppProvisionerTests() { _logger = Substitute.For(); _graph = Substitute.For(); _retryHelper = new RetryHelper(_logger, maxRetries: 1, baseDelaySeconds: 0); - _factory = new EntraAppFactory(_logger, _graph, _retryHelper); + _provisioner = new EntraAppProvisioner(_logger, _graph, _retryHelper); } [Fact] @@ -44,7 +44,7 @@ public async Task CreateProxyAppAsync_HappyPath_ReturnsAllFieldsPopulated() .Returns(Task.FromResult<(string ObjectId, string ClientId)?>((AppObjectId, AppClientId))); _graph.AddAppPasswordAsync(TenantId, AppObjectId).Returns(Task.FromResult(AppSecret)); - var result = await _factory.CreateProxyAppAsync(ServerName, TenantId, suffix: "A365Proxy", roleDisplay: "A365 Proxy", serviceTreeId: "svc-tree-7"); + var result = await _provisioner.CreateProxyAppAsync(ServerName, TenantId, suffix: "A365Proxy", roleDisplay: "A365 Proxy", serviceTreeId: "svc-tree-7"); result.Should().NotBeNull(); result!.ClientId.Should().Be(AppClientId); @@ -63,7 +63,7 @@ public async Task CreateProxyAppAsync_UsesSuffixInAppName() .Returns(Task.FromResult<(string ObjectId, string ClientId)?>((AppObjectId, AppClientId))); _graph.AddAppPasswordAsync(TenantId, AppObjectId).Returns(Task.FromResult(AppSecret)); - var result = await _factory.CreateProxyAppAsync(ServerName, TenantId, suffix: "RemoteProxy", roleDisplay: "Remote Proxy", serviceTreeId: null); + var result = await _provisioner.CreateProxyAppAsync(ServerName, TenantId, suffix: "RemoteProxy", roleDisplay: "Remote Proxy", serviceTreeId: null); result.Should().NotBeNull(); result!.AppName.Should().Be($"{ServerName}-RemoteProxy"); @@ -75,7 +75,7 @@ public async Task CreateProxyAppAsync_WhenCreateAppReturnsNull_ReturnsNullAndLog _graph.CreateEntraAppAsync(TenantId, Arg.Any(), serviceTreeId: Arg.Any(), Arg.Any()) .Returns(Task.FromResult<(string ObjectId, string ClientId)?>(null)); - var result = await _factory.CreateProxyAppAsync(ServerName, TenantId, suffix: "A365Proxy", roleDisplay: "A365 Proxy", serviceTreeId: null); + var result = await _provisioner.CreateProxyAppAsync(ServerName, TenantId, suffix: "A365Proxy", roleDisplay: "A365 Proxy", serviceTreeId: null); result.Should().BeNull(); await _graph.DidNotReceive().AddAppPasswordAsync(Arg.Any(), Arg.Any()); @@ -94,7 +94,7 @@ public async Task CreateProxyAppAsync_WhenAddPasswordReturnsNull_ReturnsNullAndL .Returns(Task.FromResult<(string ObjectId, string ClientId)?>((AppObjectId, AppClientId))); _graph.AddAppPasswordAsync(TenantId, AppObjectId).Returns(Task.FromResult(null)); - var result = await _factory.CreateProxyAppAsync(ServerName, TenantId, suffix: "A365Proxy", roleDisplay: "A365 Proxy", serviceTreeId: null); + var result = await _provisioner.CreateProxyAppAsync(ServerName, TenantId, suffix: "A365Proxy", roleDisplay: "A365 Proxy", serviceTreeId: null); result.Should().BeNull(); _logger.Received(1).Log( @@ -112,7 +112,7 @@ public async Task CreateProxyAppAsync_WhenAddPasswordReturnsWhitespace_ReturnsNu .Returns(Task.FromResult<(string ObjectId, string ClientId)?>((AppObjectId, AppClientId))); _graph.AddAppPasswordAsync(TenantId, AppObjectId).Returns(Task.FromResult(" ")); - var result = await _factory.CreateProxyAppAsync(ServerName, TenantId, suffix: "A365Proxy", roleDisplay: "A365 Proxy", serviceTreeId: null); + var result = await _provisioner.CreateProxyAppAsync(ServerName, TenantId, suffix: "A365Proxy", roleDisplay: "A365 Proxy", serviceTreeId: null); result.Should().BeNull(); } @@ -124,7 +124,7 @@ public async Task CreateProxyAppAsync_WhenClientIdIsEmpty_ReturnsNullAndLogsErro .Returns(Task.FromResult<(string ObjectId, string ClientId)?>((AppObjectId, string.Empty))); _graph.AddAppPasswordAsync(TenantId, AppObjectId).Returns(Task.FromResult(AppSecret)); - var result = await _factory.CreateProxyAppAsync(ServerName, TenantId, suffix: "RemoteProxy", roleDisplay: "Remote Proxy", serviceTreeId: null); + var result = await _provisioner.CreateProxyAppAsync(ServerName, TenantId, suffix: "RemoteProxy", roleDisplay: "Remote Proxy", serviceTreeId: null); result.Should().BeNull(); _logger.Received(1).Log( @@ -149,7 +149,7 @@ public async Task CreatePublicClientsAppAsync_HappyPath_ReturnsIdsAndSetsBrokerA .Returns(Task.FromResult(true)); var warnings = new List(); - var result = await _factory.CreatePublicClientsAppAsync(ServerName, TenantId, serviceTreeId: "svc-tree-9", warnings); + var result = await _provisioner.CreatePublicClientsAppAsync(ServerName, TenantId, serviceTreeId: "svc-tree-9", warnings); result.Should().NotBeNull(); result.ClientId.Should().Be(AppClientId); @@ -182,7 +182,7 @@ public async Task CreatePublicClientsAppAsync_WhenCreateAppReturnsNull_ReturnsAp .Returns(Task.FromResult<(string ObjectId, string ClientId)?>(null)); var warnings = new List(); - var result = await _factory.CreatePublicClientsAppAsync(ServerName, TenantId, serviceTreeId: null, warnings); + var result = await _provisioner.CreatePublicClientsAppAsync(ServerName, TenantId, serviceTreeId: null, warnings); result.ClientId.Should().BeNull(); result.ObjectId.Should().BeNull(); @@ -203,7 +203,7 @@ public async Task CreatePublicClientsAppAsync_WhenRedirectUriUpdateReturnsFalse_ .Returns(Task.FromResult(false)); var warnings = new List(); - var result = await _factory.CreatePublicClientsAppAsync(ServerName, TenantId, serviceTreeId: null, warnings); + var result = await _provisioner.CreatePublicClientsAppAsync(ServerName, TenantId, serviceTreeId: null, warnings); result.ClientId.Should().Be(AppClientId); result.ObjectId.Should().Be(AppObjectId); @@ -221,7 +221,7 @@ public async Task CreatePublicClientsAppAsync_WhenRedirectUriUpdateThrows_Return .Returns>(_ => throw new InvalidOperationException("Graph blew up")); var warnings = new List(); - var result = await _factory.CreatePublicClientsAppAsync(ServerName, TenantId, serviceTreeId: null, warnings); + var result = await _provisioner.CreatePublicClientsAppAsync(ServerName, TenantId, serviceTreeId: null, warnings); result.ClientId.Should().Be(AppClientId); result.ObjectId.Should().Be(AppObjectId); From 916a27b6f173534cd853c5a47d26623da488638b Mon Sep 17 00:00:00 2001 From: Deepali Garg Date: Wed, 3 Jun 2026 13:15:48 -0700 Subject: [PATCH 12/12] Copilot automated comments addressed --- .../Commands/PublishCommandExecutor.cs | 24 ++++-- .../Services/EntraAppProvisioner.cs | 8 ++ .../Services/IAgent365ToolingService.cs | 4 +- .../DevelopMcpCommandRegressionTests.cs | 77 +++++++++++++++++++ 4 files changed, 106 insertions(+), 7 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs index aa77c1f5..407fc31b 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs @@ -141,6 +141,14 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct } catch (Exception ex) { + // Caller cancellation (Ctrl+C) should abort fast and predictably rather than be reported + // as a publish failure and trigger rollback work. Rethrow so the process exits quickly, + // matching the cancellation handling in the setup flows. + if (ex is OperationCanceledException && ct.IsCancellationRequested) + { + throw; + } + _logger.LogError("Failed to publish MCP server '{ServerName}': {Error}", input.ServerName, ex.Message); _logger.LogDebug("Exception details: {Exception}", ex.ToString()); await RollbackEntraAppsAsync(apps, tenantId, ct); @@ -231,12 +239,14 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct // Publisher name: optional from the CLI's perspective. The platform's v2 validator // requires a non-empty value for custom (user-created) servers and ignores it for - // 1p app-based servers. Prompt the user when not supplied, but allow empty input — - // a Microsoft developer publishing msdyn_DataverseMCPServer shouldn't have to type - // anything. If they're publishing a custom server with no value, the platform's - // error message tells them what's missing. Dry-run also skips the prompt. + // 1p app-based servers. Prompt only when the option was omitted entirely (null) — a + // Microsoft developer publishing msdyn_DataverseMCPServer can just press Enter. An + // explicitly empty/whitespace value (e.g. --publisher-name "" from a script) is treated + // as "no publisher" without prompting, so non-interactive automation never hangs. If a + // custom server ends up with no value, the platform's error message says what's missing. + // Dry-run also skips the prompt. var publisherName = args.PublisherName; - if (string.IsNullOrWhiteSpace(publisherName)) + if (publisherName is null) { publisherName = args.DryRun ? null @@ -245,6 +255,10 @@ internal async Task ExecuteAsync(RawPublishArgs args, CancellationToken ct "Publisher name", maxLength: 100); } + else if (string.IsNullOrWhiteSpace(publisherName)) + { + publisherName = null; + } else { publisherName = DevelopMcpCommand.InputValidator.ValidateInput(publisherName, "Publisher name", maxLength: 100); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs index a46a36c1..14d705a0 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/EntraAppProvisioner.cs @@ -192,6 +192,14 @@ internal virtual async Task CreatePublicClientsAppAsync( } catch (Exception ex) { + // Caller cancellation (Ctrl+C) should terminate the CLI promptly rather than be demoted + // to a warning and continue. Rethrow so cancellation propagates, matching the handling + // in the command executors and setup flows. + if (ex is OperationCanceledException && ct.IsCancellationRequested) + { + throw; + } + var msg = $"Failed to set redirect URIs on Public Clients app: {ex.Message}"; _logger.LogError(msg); warnings.Add(msg); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/IAgent365ToolingService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/IAgent365ToolingService.cs index 0460f2b5..0e5df40e 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/IAgent365ToolingService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/IAgent365ToolingService.cs @@ -38,9 +38,9 @@ public interface IAgent365ToolingService /// /// Dataverse environment ID /// MCP server name to publish - /// Publish request with alias, display name, optional Entra app credentials, and optional publisher name + /// Publish request with alias, display name, the Public Clients Entra app id, and optional publisher name /// Cancellation token - /// Response from the publish operation, including the underlying server's PPMI app id and redirect URI + /// Response from the publish operation, including the underlying server's app id and OAuth scope (for the post-publish required-resource-access grant) and the echoed Public Clients app id Task PublishServerAsync( string environmentId, string serverName, diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandRegressionTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandRegressionTests.cs index d88d23ea..4b0308bd 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandRegressionTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandRegressionTests.cs @@ -235,6 +235,83 @@ public async Task PublishCommand_ForwardsParsedParametersToToolingService() "post-publish."); } + /// + /// Regression test for the empty-publisher case: an explicit --publisher-name "" (empty / + /// whitespace) must be treated as "no publisher" without triggering the interactive prompt, so + /// non-interactive automation can't hang. Exercises the non-dry-run path with --yes; the + /// proof the prompt was skipped is that the executor reaches PublishServerAsync at all (a real + /// prompt would block on Console.ReadLine), and the forwarded request carries a null publisher. + /// + [Fact] + public async Task PublishCommand_ExplicitEmptyPublisherName_SkipsPromptAndForwardsNull() + { + // Arrange + const string TestTenantId = "test-tenant-99999"; + const string TestEnvironmentId = "test-env-empty-pub"; + const string TestServerName = "msdyn_TestServer"; + const string TestAlias = "test-alias-empty-pub"; + const string TestDisplayName = "Test Display Empty Pub"; + const string TestPublicClientsObjectId = "public-clients-object-id"; + const string TestPublicClientsClientId = "public-clients-client-id"; + + var logger = Substitute.For(); + var toolingService = Substitute.For(); + var graphApiService = Substitute.For(); + + graphApiService.CreateEntraAppAsync( + TestTenantId, Arg.Any(), serviceTreeId: Arg.Any(), Arg.Any()) + .Returns(Task.FromResult<(string ObjectId, string ClientId)?>( + (TestPublicClientsObjectId, TestPublicClientsClientId))); + graphApiService.UpdateAppPublicClientRedirectUrisAsync( + TestTenantId, TestPublicClientsObjectId, Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(true)); + graphApiService.GetOAuth2PermissionScopeIdAsync( + TestTenantId, Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(Guid.NewGuid())); + graphApiService.AddRequiredResourceAccessAsync( + TestTenantId, Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(true)); + + PublishMcpServerRequest? capturedRequest = null; + toolingService.PublishServerAsync( + Arg.Any(), + Arg.Any(), + Arg.Do(r => capturedRequest = r), + Arg.Any()) + .Returns(new PublishMcpServerResponse + { + Status = "Success", + McpServerAppId = Guid.NewGuid().ToString(), + McpServerScope = "Tools.ListInvoke.All", + }); + + var executor = new TestablePublishCommandExecutor( + logger, toolingService, graphApiService, TestTenantId); + + var args = new RawPublishArgs( + EnvironmentId: TestEnvironmentId, + ServerName: TestServerName, + Alias: TestAlias, + DisplayName: TestDisplayName, + PublisherName: string.Empty, // explicit empty, e.g. --publisher-name "" from a script + Yes: true, + DryRun: false); + + // Act + var result = await executor.ExecuteAsync(args); + + // Assert + result.Should().BeTrue( + because: "an explicit empty publisher is valid (treated as no publisher), so the happy " + + "path with all dependencies mocked must return true."); + capturedRequest.Should().NotBeNull( + because: "the executor must proceed to PublishServerAsync without prompting; an explicit " + + "empty --publisher-name must not be treated as 'missing' and block on the prompt."); + capturedRequest!.PublisherName.Should().BeNull( + because: "an explicit empty/whitespace --publisher-name is normalized to null ('no " + + "publisher'); the platform decides whether that's acceptable for the server type."); + } + /// /// Test-only subclass of that stubs out /// with a known value, so the