fix: Fix crash during agent blueprint creation by improving error handling and logging in AgentBlueprintService (#366)

CHANGELOG.md

@@ -19,6 +19,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 - `a365 publish` updates manifest IDs, creates `manifest.zip`, and prints concise upload instructions for Microsoft 365 Admin Center (Agents > All agents > Upload custom agent). Interactive prompts only occur in interactive terminals; redirect stdin to suppress them in scripts.
 
 ### Fixed
+- `AgentBlueprintService.SetInheritablePermissionsAsync` no longer crashes when the Graph PATCH call throws a transient exception (#366) — the exception is caught, logged, and surfaced as a structured error result
+- `AgentBlueprintService.SetInheritablePermissionsAsync` now correctly propagates `OperationCanceledException` when the user cancels (Ctrl+C), instead of masking cancellation as a generic error
 - `A365CreateInstanceRunner` sponsor handling: sponsor is now required (Graph API rejects requests without one) — removed fallback that silently stripped the sponsor on retry, which caused `BadRequest` errors
 - Intermittent `ConnectionResetError (10054)` failures on corporate networks with TLS inspection proxies (Zscaler, Netskope) — Graph and ARM API calls now use direct MSAL.NET token acquisition instead of `az account get-access-token` subprocesses, bypassing the Python HTTP stack that triggered proxy resets (#321)
 - `a365 cleanup` blueprint deletion now succeeds for Global Administrators even when the blueprint was created by a different user

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

@@ -339,10 +339,11 @@ public virtual async Task<bool> DeleteAgentUserAsync(
         // Normalize into array form expected by Graph (each element is a single scope string)
         var desiredArray = desiredSet.ToArray();
 
+        string? blueprintObjectId = null;
         try
         {
             // Resolve blueprintId to object ID if needed
-            var blueprintObjectId = await ResolveBlueprintObjectIdAsync(tenantId, blueprintId, ct, requiredScopes);
+            blueprintObjectId = await ResolveBlueprintObjectIdAsync(tenantId, blueprintId, ct, requiredScopes);
 
             // Retrieve existing inheritable permissions
             var getPath = $"/beta/applications/microsoft.graph.agentIdentityBlueprint/{blueprintObjectId}/inheritablePermissions";
@@ -388,10 +389,22 @@ public virtual async Task<bool> DeleteAgentUserAsync(
                     }
                 };
 
-                var patched = await _graphApiService.GraphPatchAsync(tenantId, patchPath, patchPayload, ct, requiredScopes);
+                var patched = false;
+                try
+                {
+                    patched = await _graphApiService.GraphPatchAsync(tenantId, patchPath, patchPayload, ct, requiredScopes);
+                }
+                catch (Exception patchEx)
+                {
+                    if (patchEx is OperationCanceledException && ct.IsCancellationRequested) throw;
+                    _logger.LogError(patchEx, "Exception during PATCH of inheritable permissions for blueprint {Blueprint} resource {Resource}", blueprintObjectId, resourceAppId);
+                    return (ok: false, alreadyExists: false, error: patchEx.Message);
+                }
+
                 if (!patched)
                 {
-                    return (ok: false, alreadyExists: false, error: "PATCH failed");
+                    _logger.LogWarning("PATCH request to update inheritable permissions failed for blueprint {Blueprint} resource {Resource}", blueprintObjectId, resourceAppId);
+                    return (ok: false, alreadyExists: false, error: $"Graph PATCH returned false for blueprint {blueprintObjectId} resource {resourceAppId}");
                 }
 
                 _logger.LogDebug("Patched inheritable permissions for blueprint {Blueprint} resource {Resource}", blueprintObjectId, resourceAppId);
@@ -429,7 +442,8 @@ public virtual async Task<bool> DeleteAgentUserAsync(
         }
         catch (Exception ex)
         {
-            _logger.LogError("Failed to set inheritable permissions: {Error}", ex.Message);
+            if (ex is OperationCanceledException && ct.IsCancellationRequested) throw;
+            _logger.LogError(ex, "Failed to set inheritable permissions for blueprint {Blueprint} resource {Resource}: {Error}", blueprintObjectId ?? blueprintId, resourceAppId, ex.Message);
             return (ok: false, alreadyExists: false, error: ex.Message);
         }
     }

src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs

@@ -165,6 +165,100 @@ public async Task SetInheritablePermissionsAsync_Patches_WhenPresent()
         err.Should().BeNull();
     }
 
+    [Fact]
+    public async Task SetInheritablePermissionsAsync_ReturnsFalse_WhenPatchThrows()
+    {
+        // Arrange — use a subclass that overrides GraphPatchAsync to throw,
+        // simulating a transient network error during the PATCH call (#366 regression).
+        var handler = new FakeHttpMessageHandler();
+        var executor = Substitute.For<CommandExecutor>(Substitute.For<ILogger<CommandExecutor>>());
+        executor.ExecuteAsync(Arg.Any<string>(), Arg.Any<string>(), Arg.Any<string?>(), Arg.Any<bool>(), Arg.Any<bool>(), Arg.Any<CancellationToken>())
+            .Returns(callInfo =>
+            {
+                var args = callInfo.ArgAt<string>(1);
+                if (args != null && args.Contains("get-access-token", StringComparison.OrdinalIgnoreCase))
+                    return Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = "fake-token", StandardError = string.Empty });
+                return Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = "{}", StandardError = string.Empty });
+            });
+
+        var graphService = new ThrowingOnPatchGraphApiService(_mockGraphLogger, executor, FakeAuth(), handler);
+        var service = new AgentBlueprintService(_mockLogger, graphService);
+
+        // ResolveBlueprintObjectIdAsync: 404 → resolve via appId filter
+        handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.NotFound));
+        handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK)
+        {
+            Content = new StringContent(JsonSerializer.Serialize(new { value = new[] { new { id = "resolved-object-id" } } }))
+        });
+
+        // GET existing permissions — returns an entry so the merge+PATCH path is taken
+        handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK)
+        {
+            Content = new StringContent(JsonSerializer.Serialize(new
+            {
+                value = new[] { new { resourceAppId = "resAppId", inheritableScopes = new { scopes = new[] { "scope1" } } } }
+            }))
+        });
+
+        // Act
+        var (ok, already, err) = await service.SetInheritablePermissionsAsync("tid", "bpAppId", "resAppId", new[] { "scope2" });
+
+        // Assert — must not throw; must surface the failure gracefully
+        ok.Should().BeFalse(because: "GraphPatchAsync threw an exception and the caller must not crash");
+        already.Should().BeFalse();
+        err.Should().Be("Network error during PATCH");
+    }
+
+    [Fact]
+    public async Task SetInheritablePermissionsAsync_ReturnsFalse_WhenPatchReturnsFalse()
+    {
+        // Arrange — GraphPatchAsync succeeds at the HTTP level but returns a non-2xx status,
+        // causing GraphPatchAsync to return false. The method must return (ok: false) without throwing.
+        var handler = new FakeHttpMessageHandler();
+        var executor = Substitute.For<CommandExecutor>(Substitute.For<ILogger<CommandExecutor>>());
+        executor.ExecuteAsync(Arg.Any<string>(), Arg.Any<string>(), Arg.Any<string?>(), Arg.Any<bool>(), Arg.Any<bool>(), Arg.Any<CancellationToken>())
+            .Returns(callInfo =>
+            {
+                var args = callInfo.ArgAt<string>(1);
+                if (args != null && args.Contains("get-access-token", StringComparison.OrdinalIgnoreCase))
+                    return Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = "fake-token", StandardError = string.Empty });
+                return Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = "{}", StandardError = string.Empty });
+            });
+
+        var graphService = new GraphApiService(_mockGraphLogger, executor, FakeAuth(), handler, loginHintResolver: () => Task.FromResult<string?>(null));
+        var service = new AgentBlueprintService(_mockLogger, graphService);
+
+        // ResolveBlueprintObjectIdAsync: 404 → resolve via appId filter
+        handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.NotFound));
+        handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK)
+        {
+            Content = new StringContent(JsonSerializer.Serialize(new { value = new[] { new { id = "resolved-object-id" } } }))
+        });
+
+        // GET existing permissions — returns an entry so the PATCH path is taken
+        handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK)
+        {
+            Content = new StringContent(JsonSerializer.Serialize(new
+            {
+                value = new[] { new { resourceAppId = "resAppId", inheritableScopes = new { scopes = new[] { "scope1" } } } }
+            }))
+        });
+
+        // PATCH returns 400 Bad Request — GraphPatchAsync returns false
+        handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.BadRequest)
+        {
+            Content = new StringContent("{\"error\":{\"code\":\"BadRequest\",\"message\":\"Invalid payload\"}}")
+        });
+
+        // Act
+        var (ok, already, err) = await service.SetInheritablePermissionsAsync("tid", "bpAppId", "resAppId", new[] { "scope2" });
+
+        // Assert — must not throw; must surface the failure gracefully
+        ok.Should().BeFalse(because: "GraphPatchAsync returned false (HTTP 400)");
+        already.Should().BeFalse();
+        err.Should().Contain("Graph PATCH returned false", because: "error message must identify the failing operation");
+    }
+
     [Fact]
     public async Task DeleteAgentIdentityAsync_WithValidIdentity_ReturnsTrue()
     {
@@ -440,6 +534,25 @@ public async Task DeleteAgentUserAsync_ReturnsFalse_OnGraphError()
         }
     }
 
+    // Test helper — overrides GraphPatchAsync to throw, simulating a transient network failure.
+    private sealed class ThrowingOnPatchGraphApiService : GraphApiService
+    {
+        public ThrowingOnPatchGraphApiService(
+            ILogger<GraphApiService> logger,
+            CommandExecutor executor,
+            IAuthenticationService authService,
+            HttpMessageHandler handler)
+            : base(logger, executor, authService, handler) { }
+
+        public override Task<bool> GraphPatchAsync(
+            string tenantId,
+            string relativePath,
+            object payload,
+            CancellationToken ct = default,
+            IEnumerable<string>? scopes = null)
+            => Task.FromException<bool>(new HttpRequestException("Network error during PATCH"));
+    }
+
     private static IAuthenticationService FakeAuth()
     {
         var mock = Substitute.For<IAuthenticationService>();