Add manual cleanup block for unpublish

src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs

@@ -1,11 +1,13 @@
 // 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.Evaluate;
 using Microsoft.Extensions.Logging;
 using System.CommandLine;
+using System.Linq;
 
 namespace Microsoft.Agents.A365.DevTools.Cli.Commands;
 
@@ -37,7 +39,7 @@ public static Command CreateCommand(
         developMcpCommand.AddCommand(CreateListEnvironmentsSubcommand(logger, toolingService));
         developMcpCommand.AddCommand(CreateListServersSubcommand(logger, toolingService));
         developMcpCommand.AddCommand(CreatePublishSubcommand(logger, toolingService, graphApiService));
-        developMcpCommand.AddCommand(CreateUnpublishSubcommand(logger, toolingService));
+        developMcpCommand.AddCommand(CreateUnpublishSubcommand(logger, toolingService, graphApiService));
         developMcpCommand.AddCommand(CreateRegisterExternalMcpServerSubcommand(logger, toolingService, graphApiService));
 
         if (evaluationPipelineService is not null)
@@ -424,7 +426,8 @@ private static Command CreatePublishSubcommand(
     /// </summary>
     private static Command CreateUnpublishSubcommand(
         ILogger logger, 
-        IAgent365ToolingService toolingService)
+        IAgent365ToolingService toolingService,
+        GraphApiService? graphApiService)
     {
         var command = new Command("unpublish", "Unpublish an MCP server from a Dataverse environment");
 
@@ -454,9 +457,12 @@ private static Command CreateUnpublishSubcommand(
         );
         command.AddOption(verboseOption);
 
-        command.SetHandler(async (envId, serverName, dryRun, verbose) =>
+        command.SetHandler(async (context) =>
         {
-            _ = verbose;
+            var envId = context.ParseResult.GetValueForOption(envIdOption);
+            var serverName = context.ParseResult.GetValueForOption(serverNameOption);
+            var dryRun = context.ParseResult.GetValueForOption(dryRunOption);
+
             try
             {
                 // Validate and prompt for missing required arguments with security checks
@@ -466,6 +472,7 @@ private static Command CreateUnpublishSubcommand(
                     if (string.IsNullOrWhiteSpace(envId))
                     {
                         logger.LogError("Environment ID is required");
+                        context.ExitCode = 1;
                         return;
                     }
                 }
@@ -476,6 +483,7 @@ private static Command CreateUnpublishSubcommand(
                     if (envId == null)
                     {
                         logger.LogError("Invalid environment ID format");
+                        context.ExitCode = 1;
                         return;
                     }
                 }
@@ -486,6 +494,7 @@ private static Command CreateUnpublishSubcommand(
                     if (string.IsNullOrWhiteSpace(serverName))
                     {
                         logger.LogError("Server name is required");
+                        context.ExitCode = 1;
                         return;
                     }
                 }
@@ -496,13 +505,15 @@ private static Command CreateUnpublishSubcommand(
                     if (serverName == null)
                     {
                         logger.LogError("Invalid server name format");
+                        context.ExitCode = 1;
                         return;
                     }
                 }
             }
             catch (ArgumentException ex)
             {
                 logger.LogError("Input validation failed: {Message}", ex.Message);
+                context.ExitCode = 1;
                 return;
             }
 
@@ -517,21 +528,104 @@ private static Command CreateUnpublishSubcommand(
             }
 
             // Call service
-            var success = await toolingService.UnpublishServerAsync(envId, serverName);
+            var response = await toolingService.UnpublishServerAsync(envId, serverName);
 
-            if (!success)
+            if (response is null || !response.IsSuccess)
             {
                 logger.LogError("Failed to unpublish MCP server {ServerName} from environment {EnvId}", serverName, envId);
+                context.ExitCode = 1;
                 return;
             }
 
             logger.LogInformation("Successfully unpublished MCP server {ServerName} from environment {EnvId}", serverName, envId);
 
-        }, envIdOption, serverNameOption, dryRunOption, verboseOption);
+            // The platform removes the tenant publication but cannot delete Entra app registrations in the
+            // customer tenant, so it returns any it created for this server (in ManualCleanupRequired) for
+            // the CLI to clean up here.
+            await CleanupEntraAppsAsync(logger, graphApiService, response.ManualCleanupRequired?.Apps, serverName);
+        });
 
         return command;
     }
 
+    /// <summary>
+    /// Best-effort deletion of the Entra app registrations the platform returned from an unpublish. Each
+    /// delete is independent so one failure does not skip the rest, and every failure is logged with the
+    /// app id so the user can remove it manually. When Graph is unavailable or the tenant cannot be
+    /// detected, the apps are listed once for manual cleanup instead.
+    /// </summary>
+    private static async Task CleanupEntraAppsAsync(
+        ILogger logger,
+        GraphApiService? graphApiService,
+        IReadOnlyList<McpServerAppEntry>? appsToCleanup,
+        string serverName)
+    {
+        if (appsToCleanup is null || appsToCleanup.Count == 0)
+        {
+            return;
+        }
+
+        // Tenant detection shells out to az, so only attempt it when Graph is available.
+        var tenantId = graphApiService is null
+            ? null
+            : await TenantDetectionHelper.DetectTenantIdAsync(null, logger);
+
+        if (graphApiService is null || string.IsNullOrWhiteSpace(tenantId))
+        {
+            var appList = string.Join(
+                Environment.NewLine,
+                appsToCleanup.Select(app => $"  - {app.AppName ?? "<unknown>"} (appId {app.AppId ?? "<unknown>"})"));
+            logger.LogWarning(
+                "The platform could not automatically delete {Count} Entra app registration(s) for server '{ServerName}'. Delete them manually in the Azure portal:{NewLine}{AppList}",
+                appsToCleanup.Count, serverName, Environment.NewLine, appList);
+            return;
+        }
+
+        logger.LogInformation("Cleaning up Entra app registrations for unpublished server '{ServerName}'...", serverName);
+
+        foreach (var app in appsToCleanup)
+        {
+            if (string.IsNullOrWhiteSpace(app.AppId))
+            {
+                logger.LogWarning(
+                    "The platform returned Entra app '{AppName}' for cleanup without an appId, so it cannot be deleted automatically. If it exists, delete it manually in the Azure portal.",
+                    app.AppName ?? "<unknown>");
+                continue;
+            }
+
+            try
+            {
+                var objectId = await graphApiService.GetAppObjectIdByAppIdAsync(tenantId, app.AppId);
+                if (string.IsNullOrWhiteSpace(objectId))
+                {
+                    logger.LogWarning(
+                        "Entra app '{AppName}' (appId {AppId}) was not found; it may already be deleted.",
+                        app.AppName ?? "<unknown>", app.AppId);
+                    continue;
+                }
+
+                var deleted = await graphApiService.DeleteEntraAppAsync(tenantId, objectId);
+                if (deleted)
+                {
+                    logger.LogInformation("Deleted Entra app '{AppName}' (appId {AppId})", app.AppName ?? "<unknown>", app.AppId);
+                }
+                else
+                {
+                    logger.LogError(
+                        "Failed to delete Entra app '{AppName}' (appId {AppId}). Delete it manually in the Azure portal.",
+                        app.AppName ?? "<unknown>", app.AppId);
+                }
+            }
+            catch (Exception ex)
+            {
+                logger.LogError(
+                    ex,
+                    "Exception deleting Entra app '{AppName}' (appId {AppId}). Delete it manually in the Azure portal.",
+                    app.AppName ?? "<unknown>", app.AppId);
+            }
+        }
+    }
+
     /// <summary>
     /// Creates the register-external-mcp-server subcommand
     /// </summary>

src/Microsoft.Agents.A365.DevTools.Cli/Models/McpServerAppEntry.cs

@@ -0,0 +1,26 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+using System.Text.Json.Serialization;
+
+namespace Microsoft.Agents.A365.DevTools.Cli.Models;
+
+/// <summary>
+/// An Entra app registration associated with a published MCP server that the CLI is responsible for
+/// deleting on unpublish. The platform cannot delete app registrations in the customer tenant, so it
+/// returns these entries and the CLI performs the deletion using the caller's Graph permissions.
+/// </summary>
+public class McpServerAppEntry
+{
+    /// <summary>
+    /// Friendly name of the app registration (for logging / manual cleanup guidance).
+    /// </summary>
+    [JsonPropertyName("AppName")]
+    public string? AppName { get; set; }
+
+    /// <summary>
+    /// The app (client) id of the Entra app registration. The CLI resolves the underlying application
+    /// object id from this before calling Graph's application delete.
+    /// </summary>
+    [JsonPropertyName("AppId")]
+    public string? AppId { get; set; }
+}

src/Microsoft.Agents.A365.DevTools.Cli/Models/McpServerManualCleanup.cs

@@ -0,0 +1,27 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+using System.Collections.Generic;
+using System.Text.Json.Serialization;
+
+namespace Microsoft.Agents.A365.DevTools.Cli.Models;
+
+/// <summary>
+/// Describes resources the unpublish operation could not delete on the caller's behalf and that the
+/// caller must remove manually. Mirrors the platform's response contract so any client (not just this
+/// CLI) can discover the cleanup responsibility from the response body.
+/// </summary>
+public class McpServerManualCleanup
+{
+    /// <summary>
+    /// Human-readable explanation of why the listed resources were not deleted automatically and how
+    /// to remove them.
+    /// </summary>
+    [JsonPropertyName("Reason")]
+    public string? Reason { get; set; }
+
+    /// <summary>
+    /// The Entra app registrations the caller must delete manually in their own tenant.
+    /// </summary>
+    [JsonPropertyName("Apps")]
+    public List<McpServerAppEntry>? Apps { get; set; }
+}

src/Microsoft.Agents.A365.DevTools.Cli/Models/UnpublishMcpServerResponse.cs

@@ -0,0 +1,38 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+using System;
+using System.Text.Json.Serialization;
+
+namespace Microsoft.Agents.A365.DevTools.Cli.Models;
+
+/// <summary>
+/// Response model for an MCP server unpublish operation.
+/// </summary>
+public class UnpublishMcpServerResponse
+{
+    /// <summary>
+    /// Status of the unpublish operation.
+    /// </summary>
+    [JsonPropertyName("Status")]
+    public string? Status { get; set; }
+
+    /// <summary>
+    /// Message from the API response.
+    /// </summary>
+    [JsonPropertyName("Message")]
+    public string? Message { get; set; }
+
+    /// <summary>
+    /// Resources the platform could not delete in the customer tenant and that the caller must remove
+    /// manually (with a reason and the affected app registrations). Null when there is nothing for the
+    /// caller to clean up (for example OOB Dataverse servers or legacy records).
+    /// </summary>
+    [JsonPropertyName("ManualCleanupRequired")]
+    public McpServerManualCleanup? ManualCleanupRequired { get; set; }
+
+    /// <summary>
+    /// Whether the operation was successful.
+    /// </summary>
+    [JsonIgnore]
+    public bool IsSuccess => Status?.Equals("Success", StringComparison.OrdinalIgnoreCase) ?? false;
+}

src/Microsoft.Agents.A365.DevTools.Cli/Services/Agent365ToolingService.cs

@@ -553,7 +553,7 @@ private string BuildProvisionIdentityUrl(string environment, string serverName)
     }
 
     /// <inheritdoc />
-    public async Task<bool> UnpublishServerAsync(
+    public async Task<UnpublishMcpServerResponse?> UnpublishServerAsync(
         string environmentId,
         string serverName,
         CancellationToken cancellationToken = default)
@@ -587,7 +587,7 @@ public async Task<bool> UnpublishServerAsync(
             if (string.IsNullOrWhiteSpace(authToken))
             {
                 _logger.LogError("Failed to acquire authentication token");
-                return false;
+                return null;
             }
 
             // Create authenticated HTTP client
@@ -600,19 +600,41 @@ public async Task<bool> UnpublishServerAsync(
             using var response = await httpClient.DeleteAsync(endpointUrl, cancellationToken);
 
             // Validate response using common helper
-            var (isSuccess, _) = await ValidateResponseAsync(response, "unpublish MCP server", cancellationToken);
+            var (isSuccess, responseContent) = await ValidateResponseAsync(response, "unpublish MCP server", cancellationToken);
             if (!isSuccess)
             {
-                return false;
+                return null;
             }
 
             _logger.LogDebug("Successfully unpublished MCP server");
-            return true;
+
+            var unpublishResponse = string.IsNullOrWhiteSpace(responseContent)
+                ? null
+                : JsonDeserializationHelper.DeserializeWithDoubleSerialization<UnpublishMcpServerResponse>(
+                    responseContent, _logger);
+
+            // The platform always returns a JSON body on a successful unpublish, so an empty body or one
+            // that fails to parse means we could not read the response - including any ManualCleanupRequired
+            // block. The unpublish itself still succeeded, so warn the user to check for leftover Entra app
+            // registrations rather than silently returning a clean success.
+            if (unpublishResponse is null)
+            {
+                _logger.LogWarning(
+                    "The unpublish for {ServerName} succeeded but its response body was empty or could not be parsed, so any Entra app registrations the platform asked to be cleaned up may have been missed. Review the server's app registrations in the Azure portal and delete any leftovers manually.",
+                    serverName);
+                return new UnpublishMcpServerResponse
+                {
+                    Status = "Success",
+                    Message = $"Successfully unpublished {serverName}"
+                };
+            }
+
+            return unpublishResponse;
         }
         catch (Exception ex)
         {
             _logger.LogError(ex, "Failed to unpublish MCP server {ServerName} from environment {EnvId}", serverName, environmentId);
-            return false;
+            return null;
         }
     }