fix: prevent InternalServerError loop when --update-endpoint fails on create (#304)

sellakumaran · claude · web-flow · commit 80f983b74efc · 2026-03-03T10:52:08.000-08:00
* fix: prevent InternalServerError loop when --update-endpoint fails on create

When --update-endpoint fails during the CREATE step (e.g. Azure InternalServerError),
Azure may partially provision the new endpoint. Because BotMessagingEndpoint only updates
on success, all subsequent runs delete the stale old endpoint and attempt to create the
same partially-provisioned endpoint again, hitting InternalServerError on every retry.

Add Step 1.5 in UpdateEndpointAsync: pre-emptively delete the target endpoint name
(derived from newEndpointUrl) before creating it, ensuring a clean slate regardless of
any previous failed run. Only applies when NeedDeployment=false (non-Azure-hosted agents).

Co-Authored-By: Claude Sonnet 4.6 &lt;noreply@anthropic.com&gt;

* Normalize location reuse in endpoint update logic

Refactor UpdateEndpointAsync to normalize location once and reuse
for all endpoint operations, reducing duplication. Add warning
logging if pre-create cleanup does not confirm deletion. Update
unit tests to reflect normalization and endpoint name logic.
Add test to verify both old and target endpoints are deleted
when partially-provisioned endpoints exist.

* Improve logging for endpoint deletion and diagnostics

Enhanced logging for endpoint deletion by:
- Including both endpoint name and source URL in removal logs.
- Promoting endpoint name and agent blueprint ID logs to Information level.
- Logging full delete request payload details (instance name, AppId, TenantId, location, environment).
These changes improve traceability and aid in debugging endpoint deletion issues.

* Fix environment logging to use local config variable

Updated the delete request payload logging to use config.Environment
instead of _config.Environment, ensuring the correct environment
value is logged from the local configuration.

* Improve correlation ID handling and logging in cleanup

Each cleanup and blueprint command now generates and logs its own correlation ID at execution time for better traceability. The correlation ID is propagated through async operations and method calls, improving observability and debugging by ensuring all related logs are tagged with a unique identifier. Method signatures and invocations were updated to support this change.

---------

Co-authored-by: Claude Sonnet 4.6 &lt;noreply@anthropic.com&gt;
diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs
@@ -40,17 +40,18 @@ public static Command CreateCommand(
         cleanupCommand.AddOption(configOption);
         cleanupCommand.AddOption(verboseOption);
 
-        // Generate correlation ID at workflow entry point
-        var correlationId = HttpClientFactory.GenerateCorrelationId();
-
         // Set default handler for 'a365 cleanup' (without subcommand) - cleans up everything
         cleanupCommand.SetHandler(async (configFile, verbose) =>
         {
+            // Generate correlation ID at workflow entry point
+            var correlationId = HttpClientFactory.GenerateCorrelationId();
+            logger.LogInformation("Starting cleanup (CorrelationId: {CorrelationId})", correlationId);
+            
             await ExecuteAllCleanupAsync(logger, configService, botConfigurator, executor, agentBlueprintService, confirmationProvider, federatedCredentialService, configFile, correlationId: correlationId);
         }, configOption, verboseOption);
 
         // Add subcommands for granular control
-        cleanupCommand.AddCommand(CreateBlueprintCleanupCommand(logger, configService, botConfigurator, executor, agentBlueprintService, federatedCredentialService, correlationId: correlationId));
+        cleanupCommand.AddCommand(CreateBlueprintCleanupCommand(logger, configService, botConfigurator, executor, agentBlueprintService, federatedCredentialService));
         cleanupCommand.AddCommand(CreateAzureCleanupCommand(logger, configService, executor));
         cleanupCommand.AddCommand(CreateInstanceCleanupCommand(logger, configService, executor));
 
@@ -63,8 +64,7 @@ private static Command CreateBlueprintCleanupCommand(
         IBotConfigurator botConfigurator,
         CommandExecutor executor,
         AgentBlueprintService agentBlueprintService,
-        FederatedCredentialService federatedCredentialService,
-        string? correlationId = null)
+        FederatedCredentialService federatedCredentialService)
     {
         var command = new Command("blueprint", "Remove Entra ID blueprint application and service principal");
         
@@ -91,6 +91,10 @@ private static Command CreateBlueprintCleanupCommand(
         {
             try
             {
+                // Generate correlation ID at workflow entry point
+                var correlationId = HttpClientFactory.GenerateCorrelationId();
+                logger.LogInformation("Starting blueprint cleanup (CorrelationId: {CorrelationId})", correlationId);
+                
                 var config = await LoadConfigAsync(configFile, logger, configService);
                 if (config == null) return;
                 
diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs
@@ -197,7 +197,8 @@ await UpdateEndpointAsync(
                         logger: logger,
                         configService: configService,
                         botConfigurator: botConfigurator,
-                        platformDetector: platformDetector);
+                        platformDetector: platformDetector,
+                        correlationId: correlationId);
                 }
                 catch (Agent365Exception ex)
                 {
@@ -1859,13 +1860,15 @@ await ProjectSettingsSyncHelper.ExecuteAsync(
     /// <param name="configService">Configuration service</param>
     /// <param name="botConfigurator">Bot configurator service</param>
     /// <param name="platformDetector">Platform detector service</param>
+    /// <param name="correlationId">Optional correlation ID for tracing</param>
     public static async Task UpdateEndpointAsync(
         string configPath,
         string newEndpointUrl,
         ILogger logger,
         IConfigService configService,
         IBotConfigurator botConfigurator,
-        PlatformDetector platformDetector)
+        PlatformDetector platformDetector,
+        string? correlationId = null)
     {
         var setupConfig = await configService.LoadAsync(configPath);
 
@@ -1887,6 +1890,12 @@ public static async Task UpdateEndpointAsync(
         logger.LogInformation("Updating messaging endpoint...");
         logger.LogInformation("");
 
+        // Normalize location once; used by both Step 1 and Step 1.5.
+        // Null-coalescing is intentional: Location is only validated inside the Step 1 block (not here),
+        // so it may still be null at this point. The empty-string fallback is never passed to any API —
+        // Step 1 throws before using it, and Step 1.5 guards on !IsNullOrWhiteSpace(Location).
+        var normalizedLocation = setupConfig.Location?.Replace(" ", "").ToLowerInvariant() ?? string.Empty;
+
         // Step 1: Delete existing endpoint if it exists
         if (!string.IsNullOrWhiteSpace(setupConfig.MessagingEndpoint) || !string.IsNullOrWhiteSpace(setupConfig.BotName))
         {
@@ -1916,12 +1925,11 @@ public static async Task UpdateEndpointAsync(
                 endpointName = Services.Helpers.EndpointHelper.GetEndpointName(setupConfig.BotName);
             }
 
-            var normalizedLocation = setupConfig.Location.Replace(" ", "").ToLowerInvariant();
-
             var deleted = await botConfigurator.DeleteEndpointWithAgentBlueprintAsync(
                 endpointName,
                 normalizedLocation,
-                setupConfig.AgentBlueprintId);
+                setupConfig.AgentBlueprintId,
+                correlationId: correlationId);
 
             if (!deleted)
             {
@@ -1936,12 +1944,29 @@ public static async Task UpdateEndpointAsync(
             logger.LogInformation("No existing endpoint found. Proceeding with registration.");
         }
 
+        // Step 1.5: Pre-create cleanup of the target endpoint name.
+        // If a previous --update-endpoint failed during the create step, Azure may have
+        // partially provisioned the new endpoint and left it in a bad state that blocks
+        // subsequent creates with InternalServerError. Delete it now to ensure a clean slate.
+        if (!setupConfig.NeedDeployment && !string.IsNullOrWhiteSpace(setupConfig.Location))
+        {
+            var targetEndpointName = Services.Helpers.EndpointHelper.GetEndpointNameFromUrl(newEndpointUrl, setupConfig.AgentBlueprintId);
+            logger.LogInformation("Removing target endpoint '{EndpointName}' (derived from {Url}) to ensure a clean state before registration.", targetEndpointName, newEndpointUrl);
+            var preCleanupDeleted = await botConfigurator.DeleteEndpointWithAgentBlueprintAsync(targetEndpointName, normalizedLocation, setupConfig.AgentBlueprintId, correlationId: correlationId);
+            if (!preCleanupDeleted)
+            {
+                // Not fatal — proceed and let Step 2 surface the error if the partially-provisioned
+                // endpoint is still blocking. The warning helps diagnose production issues.
+                logger.LogWarning("Pre-create cleanup for '{EndpointName}' did not confirm deletion. Proceeding anyway.", targetEndpointName);
+            }
+        }
+
         // Step 2: Register new endpoint with the provided URL
         logger.LogInformation("");
         logger.LogInformation("Registering new messaging endpoint...");
 
         var (endpointRegistered, _) = await SetupHelpers.RegisterBlueprintMessagingEndpointAsync(
-            setupConfig, logger, botConfigurator, newEndpointUrl);
+            setupConfig, logger, botConfigurator, newEndpointUrl, correlationId: correlationId);
 
         if (!endpointRegistered)
         {
diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/BotConfigurator.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/BotConfigurator.cs
@@ -206,8 +206,8 @@ public async Task<bool> DeleteEndpointWithAgentBlueprintAsync(
         string? correlationId = null)
     {
         _logger.LogInformation("Deleting endpoint with Agent Blueprint Identity...");
-        _logger.LogDebug("   Endpoint Name: {EndpointName}", endpointName);
-        _logger.LogDebug("   Agent Blueprint ID: {AgentBlueprintId}", agentBlueprintId);
+        _logger.LogInformation("   Endpoint Name: {EndpointName}", endpointName);
+        _logger.LogInformation("   Agent Blueprint ID: {AgentBlueprintId}", agentBlueprintId);
 
         if (string.IsNullOrWhiteSpace(location))
         {
@@ -288,6 +288,12 @@ public async Task<bool> DeleteEndpointWithAgentBlueprintAsync(
 
                 // Call the endpoint
                 _logger.LogInformation("Making request to delete endpoint (Location: {Location}).", normalizedLocation);
+                _logger.LogInformation("Delete request payload:");
+                _logger.LogInformation("   AzureBotServiceInstanceName: {Name}", endpointName);
+                _logger.LogInformation("   AppId: {AppId}", agentBlueprintId);
+                _logger.LogInformation("   TenantId: {TenantId}", tenantId);
+                _logger.LogInformation("   Location: {Location}", normalizedLocation);
+                _logger.LogInformation("   Environment: {Environment}", EndpointHelper.GetDeploymentEnvironment(config.Environment));
 
                 using var request = new HttpRequestMessage(HttpMethod.Delete, deleteEndpointUrl);
                 request.Content = new StringContent(deleteEndpointBody.ToJsonString(), System.Text.Encoding.UTF8, "application/json");
diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BlueprintSubcommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BlueprintSubcommandTests.cs
@@ -5,6 +5,7 @@
 using Microsoft.Agents.A365.DevTools.Cli.Commands.SetupSubcommands;
 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;
 using NSubstitute;
 using System.CommandLine;
@@ -1462,7 +1463,7 @@ await _mockBotConfigurator.DidNotReceive().CreateEndpointWithAgentBlueprintAsync
     }
 
     [Fact]
-    public async Task UpdateEndpointAsync_WithNoExistingEndpoint_ShouldSkipDeleteAndRegister()
+    public async Task UpdateEndpointAsync_WithNoExistingOldEndpoint_ShouldOnlyCallPreCreateCleanup()
     {
         // Arrange - Config without BotName (no existing endpoint)
         var config = new Agent365Config
@@ -1490,6 +1491,10 @@ public async Task UpdateEndpointAsync_WithNoExistingEndpoint_ShouldSkipDeleteAnd
             _mockConfigService.SaveStateAsync(Arg.Any<Agent365Config>(), Arg.Any<string>())
                 .Returns(Task.CompletedTask);
 
+            _mockBotConfigurator.DeleteEndpointWithAgentBlueprintAsync(
+                Arg.Any<string>(), Arg.Any<string>(), Arg.Any<string>())
+                .Returns(Task.FromResult(true)); // NotFound = success for pre-create cleanup
+
             _mockBotConfigurator.CreateEndpointWithAgentBlueprintAsync(
                 Arg.Any<string>(), Arg.Any<string>(), Arg.Any<string>(), Arg.Any<string>(), Arg.Any<string>())
                 .Returns(EndpointRegistrationResult.Created);
@@ -1503,13 +1508,100 @@ await BlueprintSubcommand.UpdateEndpointAsync(
                 _mockBotConfigurator,
                 _mockPlatformDetector);
 
-            // Assert - Should NOT call delete (no existing endpoint)
-            await _mockBotConfigurator.DidNotReceive().DeleteEndpointWithAgentBlueprintAsync(
+            // Assert - Step 1 (delete old) is skipped — no existing endpoint to delete.
+            // Step 1.5 (pre-create cleanup) still calls delete exactly once with the TARGET endpoint name,
+            // so there is exactly one delete call total.
+            var expectedTargetName = EndpointHelper.GetEndpointNameFromUrl(newEndpointUrl, config.AgentBlueprintId);
+            await _mockBotConfigurator.Received(1).DeleteEndpointWithAgentBlueprintAsync(
+                expectedTargetName,
+                "eastus",
+                config.AgentBlueprintId);
+
+            // Should still register the new endpoint
+            await _mockBotConfigurator.Received(1).CreateEndpointWithAgentBlueprintAsync(
                 Arg.Any<string>(),
                 Arg.Any<string>(),
-                Arg.Any<string>());
+                newEndpointUrl,
+                Arg.Any<string>(),
+                config.AgentBlueprintId);
+        }
+        finally
+        {
+            if (File.Exists(generatedPath)) File.Delete(generatedPath);
+            if (File.Exists(configPath)) File.Delete(configPath);
+        }
+    }
 
-            // Should still register the new endpoint
+    [Fact]
+    public async Task UpdateEndpointAsync_WithExistingOldEndpointAndPartiallyProvisionedTarget_ShouldCallDeleteTwice()
+    {
+        // Regression: when a prior --update-endpoint failed during the create step, Azure may have
+        // partially provisioned the new endpoint. On the next run, BOTH Step 1 (delete old) and
+        // Step 1.5 (pre-create cleanup of target) must fire, targeting different endpoint names.
+        var currentlyRegisteredUrl = "https://currently-registered-3979.inc1.devtunnels.ms/api/messages";
+        var newEndpointUrl         = "https://newtunnel-3979.inc1.devtunnels.ms/api/messages";
+
+        var config = new Agent365Config
+        {
+            TenantId             = "00000000-0000-0000-0000-000000000000",
+            AgentBlueprintId     = "blueprint-123",
+            MessagingEndpoint    = currentlyRegisteredUrl, // static config (original tunnel)
+            BotMessagingEndpoint = currentlyRegisteredUrl, // generated config (last successful registration)
+            Location             = "eastus",
+            NeedDeployment       = false,
+            DeploymentProjectPath = Path.GetTempPath()
+        };
+
+        var testId        = Guid.NewGuid().ToString();
+        var configPath    = Path.Combine(Path.GetTempPath(), $"test-config-{testId}.json");
+        var generatedPath = Path.Combine(Path.GetTempPath(), $"a365.generated.config-{testId}.json");
+
+        await File.WriteAllTextAsync(generatedPath, "{}");
+
+        try
+        {
+            _mockConfigService.LoadAsync(Arg.Any<string>(), Arg.Any<string>())
+                .Returns(Task.FromResult(config));
+
+            _mockConfigService.SaveStateAsync(Arg.Any<Agent365Config>(), Arg.Any<string>())
+                .Returns(Task.CompletedTask);
+
+            _mockBotConfigurator.DeleteEndpointWithAgentBlueprintAsync(
+                Arg.Any<string>(), Arg.Any<string>(), Arg.Any<string>())
+                .Returns(Task.FromResult(true));
+
+            _mockBotConfigurator.CreateEndpointWithAgentBlueprintAsync(
+                Arg.Any<string>(), Arg.Any<string>(), Arg.Any<string>(), Arg.Any<string>(), Arg.Any<string>())
+                .Returns(EndpointRegistrationResult.Created);
+
+            // Act
+            await BlueprintSubcommand.UpdateEndpointAsync(
+                configPath,
+                newEndpointUrl,
+                _mockLogger,
+                _mockConfigService,
+                _mockBotConfigurator,
+                _mockPlatformDetector);
+
+            // Assert — exactly two delete calls with distinct endpoint names
+            var oldEndpointName    = EndpointHelper.GetEndpointNameFromUrl(currentlyRegisteredUrl, config.AgentBlueprintId);
+            var targetEndpointName = EndpointHelper.GetEndpointNameFromUrl(newEndpointUrl, config.AgentBlueprintId);
+
+            oldEndpointName.Should().NotBe(targetEndpointName, "Step 1 and Step 1.5 must target different endpoints");
+
+            // Step 1: delete the currently-registered (old) endpoint
+            await _mockBotConfigurator.Received(1).DeleteEndpointWithAgentBlueprintAsync(
+                oldEndpointName, "eastus", config.AgentBlueprintId);
+
+            // Step 1.5: pre-create cleanup of the partially-provisioned target endpoint
+            await _mockBotConfigurator.Received(1).DeleteEndpointWithAgentBlueprintAsync(
+                targetEndpointName, "eastus", config.AgentBlueprintId);
+
+            // Total: exactly two delete calls
+            await _mockBotConfigurator.Received(2).DeleteEndpointWithAgentBlueprintAsync(
+                Arg.Any<string>(), Arg.Any<string>(), Arg.Any<string>());
+
+            // Step 2: register the new endpoint
             await _mockBotConfigurator.Received(1).CreateEndpointWithAgentBlueprintAsync(
                 Arg.Any<string>(),
                 Arg.Any<string>(),
diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Helpers/EndpointHelperTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Helpers/EndpointHelperTests.cs
@@ -165,6 +165,23 @@ public void GetEndpointNameFromUrl_WithNgrokUrl_ReturnsExpectedName()
         result.Should().Be(expected);
     }
 
+    [Fact]
+    public void GetEndpointNameFromUrl_DifferentDevTunnelUrls_ProduceDifferentNames()
+    {
+        // Regression: second --update-endpoint must delete the currently registered endpoint,
+        // not the original MessagingEndpoint. Different URLs must yield different names so
+        // the delete step targets the right Azure resource.
+        var originalUrl = "https://x23kz7ll-3979.inc1.devtunnels.ms/api/messages"; // MessagingEndpoint (static)
+        var updatedUrl  = "https://w3jv62pr-3979.inc1.devtunnels.ms/api/messages"; // BotMessagingEndpoint (after first update)
+        var blueprintId = "e412c0fa-7127-4615-9bef-1a7ba99e76af";
+
+        var originalName = EndpointHelper.GetEndpointNameFromUrl(originalUrl, blueprintId);
+        var updatedName  = EndpointHelper.GetEndpointNameFromUrl(updatedUrl,  blueprintId);
+
+        updatedName.Should().NotBe(originalName, "deletion must target the currently registered URL, not the static MessagingEndpoint");
+        updatedName.Should().Be("w3jv62pr-3979-inc1-devtunnels-ms-e412c0fa");
+    }
+
     // GetEndpointNameFromHost tests
 
     [Fact]
