From 13dc5f07164026f0a6929af7feca135f1123a7ab Mon Sep 17 00:00:00 2001 From: Grant Harris Date: Tue, 24 Feb 2026 00:29:51 -0800 Subject: [PATCH 01/15] fix for orphaned agent instances in a365 cleanup blueprint --- .../Commands/CleanupCommand.cs | 140 +++++++++++-- .../Models/AgentInstanceInfo.cs | 20 ++ .../Services/AgentBlueprintService.cs | 118 ++++++++++- .../Commands/BlueprintSubcommandTests.cs | 10 +- .../CleanupCommandBotEndpointTests.cs | 7 +- .../Commands/CleanupCommandTests.cs | 193 +++++++++++++++++- .../Services/AgentBlueprintServiceTests.cs | 138 +++++++++++++ 7 files changed, 587 insertions(+), 39 deletions(-) create mode 100644 src/Microsoft.Agents.A365.DevTools.Cli/Models/AgentInstanceInfo.cs diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs index 8b4c3bd4..e4e5a8c2 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs @@ -49,7 +49,7 @@ public static Command CreateCommand( }, 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, confirmationProvider, federatedCredentialService, correlationId: correlationId)); cleanupCommand.AddCommand(CreateAzureCleanupCommand(logger, configService, executor)); cleanupCommand.AddCommand(CreateInstanceCleanupCommand(logger, configService, executor)); @@ -62,6 +62,7 @@ private static Command CreateBlueprintCleanupCommand( IBotConfigurator botConfigurator, CommandExecutor executor, AgentBlueprintService agentBlueprintService, + IConfirmationProvider confirmationProvider, FederatedCredentialService federatedCredentialService, string? correlationId = null) { @@ -106,41 +107,105 @@ private static Command CreateBlueprintCleanupCommand( return; } - // Full blueprint cleanup (original behavior) + // Full blueprint cleanup with cascade instance deletion logger.LogInformation("Starting blueprint cleanup..."); - // Check if there's actually a blueprint to clean up if (string.IsNullOrWhiteSpace(config.AgentBlueprintId)) { logger.LogInformation("No blueprint application found to clean up"); return; } + // Query for agent instances linked to this blueprint before showing preview + logger.LogInformation("Querying for agent instances linked to blueprint..."); + var instances = await agentBlueprintService.GetAgentInstancesForBlueprintAsync( + config.TenantId, + config.AgentBlueprintId); + + // Show preview logger.LogInformation(""); logger.LogInformation("Blueprint Cleanup Preview:"); logger.LogInformation("============================="); logger.LogInformation("Will delete Entra ID application: {BlueprintId}", config.AgentBlueprintId); logger.LogInformation(" Name: {DisplayName}", config.AgentBlueprintDisplayName); + + if (instances.Count > 0) + { + logger.LogInformation(""); + logger.LogInformation("Will also delete {Count} agent instance(s) linked to this blueprint:", instances.Count); + foreach (var instance in instances) + { + logger.LogInformation(" Instance: {DisplayName} (SP: {SpId})", instance.DisplayName ?? "(unnamed)", instance.IdentitySpId); + if (!string.IsNullOrWhiteSpace(instance.AgentUserId)) + logger.LogInformation(" Agentic user: {UserId}", instance.AgentUserId); + } + } + logger.LogInformation(""); - Console.Write("Continue with blueprint cleanup? (y/N): "); - var response = Console.ReadLine()?.Trim().ToLowerInvariant(); - if (response != "y" && response != "yes") + if (!await confirmationProvider.ConfirmAsync("Continue with blueprint cleanup? (y/N): ")) { logger.LogInformation("Cleanup cancelled by user"); return; } + // Delete instances first (warn and continue on failure) + var failedResources = new Dictionary> + { + ["agentic users"] = new List(), + ["identity SPs"] = new List() + }; + + foreach (var instance in instances) + { + // Delete agentic user before identity SP + if (!string.IsNullOrWhiteSpace(instance.AgentUserId)) + { + logger.LogInformation("Deleting agentic user {UserId} for instance {DisplayName}...", + instance.AgentUserId, instance.DisplayName ?? instance.IdentitySpId); + + var userDeleted = await agentBlueprintService.DeleteAgentUserAsync( + config.TenantId, + instance.AgentUserId); + + if (!userDeleted) + { + logger.LogWarning("Failed to delete agentic user {UserId} -- will continue", instance.AgentUserId); + failedResources["agentic users"].Add(instance.AgentUserId!); + } + else + { + logger.LogInformation("Agentic user deleted"); + } + } + + // Delete identity SP + logger.LogInformation("Deleting agent identity SP {SpId} for instance {DisplayName}...", + instance.IdentitySpId, instance.DisplayName ?? instance.IdentitySpId); + + var spDeleted = await agentBlueprintService.DeleteAgentIdentityAsync( + config.TenantId, + instance.IdentitySpId); + + if (!spDeleted) + { + logger.LogWarning("Failed to delete agent identity SP {SpId} -- will continue", instance.IdentitySpId); + failedResources["identity SPs"].Add(instance.IdentitySpId); + } + else + { + logger.LogInformation("Agent identity SP deleted"); + } + } + // Delete federated credentials first before deleting the blueprint logger.LogInformation(""); logger.LogInformation("Deleting federated credentials from blueprint..."); - + // Configure FederatedCredentialService with custom client app ID if available if (!string.IsNullOrWhiteSpace(config.ClientAppId)) - { federatedCredentialService.CustomClientAppId = config.ClientAppId; - } - + var ficsDeleted = await federatedCredentialService.DeleteAllFederatedCredentialsAsync( config.TenantId, config.AgentBlueprintId); @@ -155,28 +220,26 @@ private static Command CreateBlueprintCleanupCommand( logger.LogInformation("Federated credentials deleted successfully"); } - // Delete the agent blueprint using the special Graph API endpoint + // Delete the agent blueprint logger.LogInformation(""); logger.LogInformation("Deleting agent blueprint application..."); var deleted = await agentBlueprintService.DeleteAgentBlueprintAsync( config.TenantId, config.AgentBlueprintId); - + if (!deleted) { - logger.LogWarning(""); + Console.WriteLine(); logger.LogWarning("Blueprint deletion failed."); + PrintOrphanSummary(logger, failedResources); return; } - // Blueprint deleted successfully logger.LogInformation("Agent blueprint application deleted successfully"); // Handle endpoint deletion if needed using shared helper if (!await DeleteMessagingEndpointAsync(logger, config, botConfigurator, correlationId: correlationId)) - { return; - } // Clear configuration after successful blueprint deletion logger.LogInformation(""); @@ -188,8 +251,17 @@ private static Command CreateBlueprintCleanupCommand( await configService.SaveStateAsync(config); logger.LogInformation("Local configuration cleared"); - logger.LogInformation(""); - logger.LogInformation("Blueprint cleanup completed successfully!"); + + // Emit orphan summary if any instance deletions failed + if (HasOrphanedResources(failedResources)) + { + PrintOrphanSummary(logger, failedResources); + } + else + { + logger.LogInformation(""); + logger.LogInformation("Blueprint cleanup completed successfully!"); + } } catch (Exception ex) { @@ -808,6 +880,38 @@ private static async Task ExecuteEndpointOnlyCleanupAsync( logger.LogInformation(""); } + /// + /// Checks whether any instance deletions were recorded as failures. + /// + private static bool HasOrphanedResources(Dictionary> failedResources) + { + return failedResources["agentic users"].Count + failedResources["identity SPs"].Count > 0; + } + + /// + /// Prints a summary of orphaned Entra ID resources that could not be deleted. + /// This should be called whenever instance deletions have failed, regardless of + /// whether the blueprint deletion itself succeeded or failed. + /// + private static void PrintOrphanSummary( + ILogger logger, + Dictionary> failedResources) + { + if (!HasOrphanedResources(failedResources)) + { + return; + } + + Console.WriteLine(); + logger.LogWarning("Blueprint cleanup completed with warnings."); + logger.LogWarning("The following resources could not be deleted and remain orphaned in Entra ID:"); + foreach (var userId in failedResources["agentic users"]) + logger.LogWarning(" Orphaned agentic user: {ResourceId}", userId); + foreach (var spId in failedResources["identity SPs"]) + logger.LogWarning(" Orphaned identity SP: {ResourceId}", spId); + logger.LogWarning("Delete them manually via the Entra portal or Graph API."); + } + private static async Task LoadConfigAsync( FileInfo? configFile, ILogger logger, diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Models/AgentInstanceInfo.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Models/AgentInstanceInfo.cs new file mode 100644 index 00000000..86507fef --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Models/AgentInstanceInfo.cs @@ -0,0 +1,20 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +namespace Microsoft.Agents.A365.DevTools.Cli.Models; + +/// +/// Represents an agent instance linked to a blueprint, consisting of an agent identity +/// service principal and an optional agentic user. +/// +public sealed record AgentInstanceInfo +{ + /// Graph object ID of the agent identity service principal. + public required string IdentitySpId { get; init; } + + /// Display name of the identity service principal, shown in cleanup preview. + public string? DisplayName { get; init; } + + /// Graph object ID of the linked agentic user, if one exists. + public string? AgentUserId { get; init; } +} diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs index fa973f54..bdf37285 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs @@ -77,7 +77,7 @@ public string? CustomClientAppId /// The blueprint application ID (object ID or app ID) /// Cancellation token /// True if deletion succeeded or resource not found; false otherwise - public async Task DeleteAgentBlueprintAsync( + public virtual async Task DeleteAgentBlueprintAsync( string tenantId, string blueprintId, CancellationToken cancellationToken = default) @@ -129,14 +129,14 @@ public async Task DeleteAgentBlueprintAsync( /// The unique identifier of the agent identity application to delete. /// A cancellation token that can be used to cancel the delete operation. /// True if deletion succeeded or resource not found; false otherwise - public async Task DeleteAgentIdentityAsync( + public virtual async Task DeleteAgentIdentityAsync( string tenantId, string applicationId, CancellationToken cancellationToken = default) { try { - _logger.LogInformation("Deleting agent identity application: {applicationId}", applicationId); + _logger.LogInformation("Deleting agent identity application: {ApplicationId}", applicationId); // Agent Identity deletion requires special delegated permission scope var requiredScopes = new[] { "AgentIdentityBlueprint.ReadWrite.All" }; @@ -162,6 +162,118 @@ public async Task DeleteAgentIdentityAsync( } } + /// + /// Queries Entra ID for all agent identity service principals linked to the given blueprint. + /// Returns an empty list if the query fails or no instances are found. + /// + /// The tenant ID for authentication. + /// The blueprint application ID or object ID. + /// Cancellation token. + /// List of agent instances linked to the blueprint. + public virtual async Task> GetAgentInstancesForBlueprintAsync( + string tenantId, + string blueprintId, + CancellationToken cancellationToken = default) + { + try + { + // NOTE: "identityParentId" and "agentUserId" are placeholder property names. + // Verify the actual Graph beta API response shape before releasing. + var requiredScopes = new[] { "AgentIdentityBlueprint.ReadWrite.All" }; + using var doc = await _graphApiService.GraphGetAsync( + tenantId, + "/beta/servicePrincipals/microsoft.graph.agentIdentity", + cancellationToken, + requiredScopes); + + if (doc is null) + { + _logger.LogWarning("Failed to retrieve agent instances from Graph API"); + return Array.Empty(); + } + + var results = new List(); + + if (!doc.RootElement.TryGetProperty("value", out var valueArray) || + valueArray.ValueKind != JsonValueKind.Array) + { + return Array.Empty(); + } + + foreach (var item in valueArray.EnumerateArray()) + { + var parentId = item.TryGetProperty("identityParentId", out var p) ? p.GetString() : null; + if (!string.Equals(parentId, blueprintId, StringComparison.OrdinalIgnoreCase)) + { + continue; + } + + var spId = item.TryGetProperty("id", out var id) ? id.GetString() : null; + if (string.IsNullOrWhiteSpace(spId)) + { + continue; + } + + var displayName = item.TryGetProperty("displayName", out var dn) ? dn.GetString() : null; + var agentUserId = item.TryGetProperty("agentUserId", out var uid) ? uid.GetString() : null; + + results.Add(new AgentInstanceInfo + { + IdentitySpId = spId, + DisplayName = displayName, + AgentUserId = string.IsNullOrWhiteSpace(agentUserId) ? null : agentUserId + }); + } + + return results; + } + catch (Exception ex) + { + _logger.LogError(ex, "Exception querying agent instances for blueprint {BlueprintId}", blueprintId); + return Array.Empty(); + } + } + + /// + /// Deletes an agentic user from Entra ID using the agentUsers beta endpoint. + /// + /// The tenant ID for authentication. + /// The object ID of the agentic user to delete. + /// Cancellation token. + /// True if deletion succeeded or user was not found; false on error. + public virtual async Task DeleteAgentUserAsync( + string tenantId, + string agentUserId, + CancellationToken cancellationToken = default) + { + try + { + _logger.LogInformation("Deleting agentic user: {AgentUserId}", agentUserId); + + var requiredScopes = new[] { "AgentIdentityBlueprint.ReadWrite.All" }; + var deletePath = $"/beta/agentUsers/{agentUserId}"; + + var success = await _graphApiService.GraphDeleteAsync( + tenantId, + deletePath, + cancellationToken, + treatNotFoundAsSuccess: true, + scopes: requiredScopes); + + if (success) + _logger.LogInformation("Agentic user deleted successfully"); + else + _logger.LogError("Failed to delete agentic user: {AgentUserId}", agentUserId); + + return success; + } + catch (Exception ex) + { + _logger.LogError(ex, "Exception deleting agentic user: {AgentUserId}", agentUserId); + return false; + } + } + /// /// Sets inheritable permissions for an agent blueprint with proper scope merging. /// Checks if permissions already exist and merges scopes if needed via PATCH. 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 index b9b903a7..a96692a9 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BlueprintSubcommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BlueprintSubcommandTests.cs @@ -1345,7 +1345,7 @@ public async Task UpdateEndpointAsync_WithValidUrl_ShouldDeleteAndRegisterEndpoi .Returns(Task.CompletedTask); _mockBotConfigurator.DeleteEndpointWithAgentBlueprintAsync( - Arg.Any(), Arg.Any(), Arg.Any()) + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) .Returns(true); _mockBotConfigurator.CreateEndpointWithAgentBlueprintAsync( @@ -1365,7 +1365,8 @@ await BlueprintSubcommand.UpdateEndpointAsync( await _mockBotConfigurator.Received(1).DeleteEndpointWithAgentBlueprintAsync( Arg.Any(), Arg.Any(), - config.AgentBlueprintId); + config.AgentBlueprintId, + Arg.Any()); await _mockBotConfigurator.Received(1).CreateEndpointWithAgentBlueprintAsync( Arg.Any(), @@ -1435,7 +1436,7 @@ public async Task UpdateEndpointAsync_WhenDeleteFails_ShouldThrowAndNotRegister( .Returns(Task.FromResult(config)); _mockBotConfigurator.DeleteEndpointWithAgentBlueprintAsync( - Arg.Any(), Arg.Any(), Arg.Any()) + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) .Returns(false); // Delete fails // Act & Assert @@ -1505,7 +1506,8 @@ await BlueprintSubcommand.UpdateEndpointAsync( await _mockBotConfigurator.DidNotReceive().DeleteEndpointWithAgentBlueprintAsync( Arg.Any(), Arg.Any(), - Arg.Any()); + Arg.Any(), + Arg.Any()); // Should still register the new endpoint await _mockBotConfigurator.Received(1).CreateEndpointWithAgentBlueprintAsync( diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandBotEndpointTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandBotEndpointTests.cs index 3ee26492..52ca8f69 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandBotEndpointTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandBotEndpointTests.cs @@ -47,9 +47,10 @@ public CleanupCommandBotEndpointTests() _mockBotConfigurator = Substitute.For(); _mockBotConfigurator.DeleteEndpointWithAgentBlueprintAsync( - Arg.Any(), - Arg.Any(), - Arg.Any()) + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any()) .Returns(Task.FromResult(true)); _mockTokenProvider = Substitute.For(); diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandTests.cs index d18c06dd..56f77cca 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandTests.cs @@ -254,24 +254,195 @@ public void CleanupSubcommands_ShouldHaveRequiredOptions() Assert.DoesNotContain("force", optionNames); } - [Fact(Skip = "Requires interactive confirmation. Refactor command to allow test automation.")] + [Fact] public async Task CleanupBlueprint_WithValidConfig_ShouldReturnSuccess() { // Arrange var config = CreateValidConfig(); _mockConfigService.LoadAsync(Arg.Any(), Arg.Any()).Returns(config); + _mockBotConfigurator.DeleteEndpointWithAgentBlueprintAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(true); + _mockConfirmationProvider.ConfirmAsync(Arg.Any()).Returns(true); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); + var stubbedBlueprintService = CreateStubbedBlueprintService(); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, stubbedBlueprintService, _mockConfirmationProvider, _federatedCredentialService); var args = new[] { "cleanup", "blueprint", "--config", "test.json" }; // Act var result = await command.InvokeAsync(args); // Assert - Assert.Equal(0, result); // Success exit code - - // Test behavior: Blueprint cleanup currently succeeds (placeholder implementation) - // When actual PowerShell integration is added, this test can be enhanced + result.Should().Be(0); + } + + private AgentBlueprintService CreateStubbedBlueprintService( + IReadOnlyList? instances = null, + bool deleteUserResult = true, + bool deleteIdentityResult = true, + bool deleteBlueprintResult = true) + { + var mockBlueprintLogger = Substitute.For>(); + var spyService = Substitute.ForPartsOf(mockBlueprintLogger, _graphApiService); + + spyService.GetAgentInstancesForBlueprintAsync(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(instances ?? (IReadOnlyList)Array.Empty()); + + spyService.DeleteAgentUserAsync(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(deleteUserResult); + + spyService.DeleteAgentIdentityAsync(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(deleteIdentityResult); + + spyService.DeleteAgentBlueprintAsync(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(deleteBlueprintResult); + + return spyService; + } + + /// + /// Verifies that blueprint cleanup deletes agent instances before deleting the blueprint. + /// Instance deletion order: agentic user first, then identity SP, then blueprint. + /// + [Fact] + public async Task CleanupBlueprint_WithInstances_DeletesInstancesBeforeBlueprint() + { + // Arrange + var config = CreateValidConfig(); + // Capture blueprint ID before the command clears it during config save + var expectedBlueprintId = config.AgentBlueprintId!; + _mockConfigService.LoadAsync(Arg.Any(), Arg.Any()).Returns(config); + _mockBotConfigurator.DeleteEndpointWithAgentBlueprintAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(true); + + var instances = new List + { + new() { IdentitySpId = "sp-id-1", DisplayName = "Instance A", AgentUserId = "user-id-1" } + }; + var spyService = CreateStubbedBlueprintService(instances: instances); + + _mockConfirmationProvider.ConfirmAsync(Arg.Any()).Returns(true); + + var command = CleanupCommand.CreateCommand( + _mockLogger, _mockConfigService, _mockBotConfigurator, + _mockExecutor, spyService, _mockConfirmationProvider, _federatedCredentialService); + var args = new[] { "cleanup", "blueprint", "--config", "test.json" }; + + // Act + var result = await command.InvokeAsync(args); + + // Assert + result.Should().Be(0); + + await spyService.Received(1).DeleteAgentUserAsync( + config.TenantId, "user-id-1", Arg.Any()); + + await spyService.Received(1).DeleteAgentIdentityAsync( + config.TenantId, "sp-id-1", Arg.Any()); + + await spyService.Received(1).DeleteAgentBlueprintAsync( + config.TenantId, expectedBlueprintId, Arg.Any()); + } + + /// + /// Verifies that blueprint cleanup with no instances proceeds exactly as before + /// (no instance deletion calls made). + /// + [Fact] + public async Task CleanupBlueprint_WithNoInstances_ProceedsAsNormal() + { + // Arrange + var config = CreateValidConfig(); + // Capture blueprint ID before the command clears it during config save + var expectedBlueprintId = config.AgentBlueprintId!; + _mockConfigService.LoadAsync(Arg.Any(), Arg.Any()).Returns(config); + _mockBotConfigurator.DeleteEndpointWithAgentBlueprintAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(true); + + var spyService = CreateStubbedBlueprintService(instances: Array.Empty()); + + _mockConfirmationProvider.ConfirmAsync(Arg.Any()).Returns(true); + + var command = CleanupCommand.CreateCommand( + _mockLogger, _mockConfigService, _mockBotConfigurator, + _mockExecutor, spyService, _mockConfirmationProvider, _federatedCredentialService); + var args = new[] { "cleanup", "blueprint", "--config", "test.json" }; + + // Act + var result = await command.InvokeAsync(args); + + // Assert + result.Should().Be(0); + + await spyService.DidNotReceive().DeleteAgentUserAsync( + Arg.Any(), Arg.Any(), Arg.Any()); + await spyService.DidNotReceive().DeleteAgentIdentityAsync( + Arg.Any(), Arg.Any(), Arg.Any()); + + await spyService.Received(1).DeleteAgentBlueprintAsync( + config.TenantId, expectedBlueprintId, Arg.Any()); + } + + /// + /// Verifies that when an instance deletion fails, a warning is emitted and the + /// blueprint is still deleted (warn-and-continue behaviour). + /// + [Fact] + public async Task CleanupBlueprint_InstanceDeletionFails_WarnsAndContinuesToBlueprint() + { + // Arrange + var config = CreateValidConfig(); + // Capture blueprint ID before the command clears it during config save + var expectedBlueprintId = config.AgentBlueprintId!; + _mockConfigService.LoadAsync(Arg.Any(), Arg.Any()).Returns(config); + _mockBotConfigurator.DeleteEndpointWithAgentBlueprintAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(true); + + var instances = new List + { + new() { IdentitySpId = "sp-id-1", DisplayName = "Instance A", AgentUserId = "user-id-1" } + }; + var spyService = CreateStubbedBlueprintService( + instances: instances, + deleteUserResult: false, + deleteIdentityResult: true, + deleteBlueprintResult: true); + + _mockConfirmationProvider.ConfirmAsync(Arg.Any()).Returns(true); + + var command = CleanupCommand.CreateCommand( + _mockLogger, _mockConfigService, _mockBotConfigurator, + _mockExecutor, spyService, _mockConfirmationProvider, _federatedCredentialService); + var args = new[] { "cleanup", "blueprint", "--config", "test.json" }; + + // Act + var result = await command.InvokeAsync(args); + + // Assert -- command succeeds overall + result.Should().Be(0); + + // Blueprint is still deleted despite the instance failure + await spyService.Received(1).DeleteAgentBlueprintAsync( + config.TenantId, expectedBlueprintId, Arg.Any()); + + // Verify a warning was logged about the failed agentic user deletion + _mockLogger.Received().Log( + LogLevel.Warning, + Arg.Any(), + Arg.Is(o => o.ToString()!.Contains("Failed to delete agentic user") && o.ToString()!.Contains("user-id-1")), + Arg.Any(), + Arg.Any>()); + + // Verify the orphan summary warning was emitted for the failed resource + _mockLogger.Received().Log( + LogLevel.Warning, + Arg.Any(), + Arg.Is(o => o.ToString()!.Contains("Orphaned agentic user") && o.ToString()!.Contains("user-id-1")), + Arg.Any(), + Arg.Any>()); } private static Agent365Config CreateValidConfig() @@ -501,7 +672,7 @@ public async Task CleanupBlueprint_WithEndpointOnlyAndNoBlueprintId_ShouldLogErr // Verify no deletion operations were called (because blueprint ID is missing) await _mockBotConfigurator.DidNotReceive().DeleteEndpointWithAgentBlueprintAsync( - Arg.Any(), Arg.Any(), Arg.Any()); + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()); } /// @@ -539,7 +710,7 @@ public async Task CleanupBlueprint_WithEndpointOnlyAndNoBotName_ShouldLogInfo() // Verify no deletion operations were called (because BotName is empty) await _mockBotConfigurator.DidNotReceive().DeleteEndpointWithAgentBlueprintAsync( - Arg.Any(), Arg.Any(), Arg.Any()); + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()); } /// @@ -673,7 +844,7 @@ public async Task CleanupBlueprint_WithEndpointOnlyAndWhitespaceBlueprint_Should // Verify no deletion operations were called since blueprint ID is invalid await _mockBotConfigurator.DidNotReceive().DeleteEndpointWithAgentBlueprintAsync( - Arg.Any(), Arg.Any(), Arg.Any()); + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()); } /// @@ -764,7 +935,7 @@ public async Task CleanupBlueprint_WithEndpointOnlyAndEmptyInput_ShouldCancelCle // Arrange var config = CreateValidConfig(); _mockConfigService.LoadAsync(Arg.Any(), Arg.Any()).Returns(config); - _mockBotConfigurator.DeleteEndpointWithAgentBlueprintAsync(Arg.Any(), Arg.Any(), Arg.Any()) + _mockBotConfigurator.DeleteEndpointWithAgentBlueprintAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) .Returns(true); var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); @@ -785,7 +956,7 @@ public async Task CleanupBlueprint_WithEndpointOnlyAndEmptyInput_ShouldCancelCle // Verify NO deletion was called because empty input defaults to cancel await _mockBotConfigurator.DidNotReceive().DeleteEndpointWithAgentBlueprintAsync( - Arg.Any(), Arg.Any(), Arg.Any()); + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()); } finally { diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs index 80fcdb73..01431806 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs @@ -5,6 +5,7 @@ using System.Net.Http; using System.Text.Json; using FluentAssertions; +using Microsoft.Agents.A365.DevTools.Cli.Models; using Microsoft.Agents.A365.DevTools.Cli.Services; using Microsoft.Extensions.Logging; using NSubstitute; @@ -322,6 +323,133 @@ public async Task DeleteAgentIdentityAsync_WhenExceptionThrown_ReturnsFalse() Arg.Any(), Arg.Any>()); } + + [Fact] + public async Task GetAgentInstancesForBlueprintAsync_ReturnsFilteredInstances() + { + // Arrange + var handler = new FakeHttpMessageHandler(); + var executor = Substitute.For(Substitute.For>()); + executor.ExecuteAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(callInfo => + { + var args = callInfo.ArgAt(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 = string.Empty, StandardError = string.Empty }); + }); + + _mockTokenProvider.GetMgGraphAccessTokenAsync( + Arg.Any(), Arg.Any>(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns("test-token"); + + var graphService = new GraphApiService(_mockGraphLogger, executor, handler, _mockTokenProvider); + var service = new AgentBlueprintService(_mockLogger, graphService); + + const string blueprintId = "bp-id-123"; + + // Returns two SPs; only one matches the blueprint + // NOTE: "identityParentId" and "agentUserId" are placeholder property names pending live API verification + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent(JsonSerializer.Serialize(new + { + value = new[] + { + new { id = "sp-obj-1", displayName = "Instance A", identityParentId = blueprintId, agentUserId = "user-obj-1" }, + new { id = "sp-obj-2", displayName = "Instance B", identityParentId = "other-blueprint-id", agentUserId = "user-obj-2" } + } + })) + }); + + // Act + var instances = await service.GetAgentInstancesForBlueprintAsync("tenant-id", blueprintId); + + // Assert + instances.Should().HaveCount(1); + instances[0].IdentitySpId.Should().Be("sp-obj-1"); + instances[0].DisplayName.Should().Be("Instance A"); + instances[0].AgentUserId.Should().Be("user-obj-1"); + } + + [Fact] + public async Task GetAgentInstancesForBlueprintAsync_ReturnsEmpty_WhenNoneFound() + { + // Arrange + var handler = new FakeHttpMessageHandler(); + var executor = Substitute.For(Substitute.For>()); + executor.ExecuteAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(callInfo => + { + var args = callInfo.ArgAt(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 = string.Empty, StandardError = string.Empty }); + }); + + _mockTokenProvider.GetMgGraphAccessTokenAsync( + Arg.Any(), Arg.Any>(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns("test-token"); + + var graphService = new GraphApiService(_mockGraphLogger, executor, handler, _mockTokenProvider); + var service = new AgentBlueprintService(_mockLogger, graphService); + + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent(JsonSerializer.Serialize(new { value = Array.Empty() })) + }); + + // Act + var instances = await service.GetAgentInstancesForBlueprintAsync("tenant-id", "bp-id-123"); + + // Assert + instances.Should().BeEmpty(); + } + + [Fact] + public async Task DeleteAgentUserAsync_ReturnsTrue_OnSuccess() + { + // Arrange + var handler = new FakeHttpMessageHandler(); + var executor = Substitute.For(Substitute.For>()); + + _mockTokenProvider.GetMgGraphAccessTokenAsync( + Arg.Any(), Arg.Any>(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns("test-token"); + + var graphService = new GraphApiService(_mockGraphLogger, executor, handler, _mockTokenProvider); + var service = new AgentBlueprintService(_mockLogger, graphService); + + // Queue HTTP response for DELETE /beta/agentUsers/{userId} + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.NoContent)); + + // Act + var result = await service.DeleteAgentUserAsync("tenant-id", "user-obj-1"); + + // Assert + result.Should().BeTrue(); + } + + [Fact] + public async Task DeleteAgentUserAsync_ReturnsFalse_OnGraphError() + { + // Arrange + var handler = new FakeHttpMessageHandler(); + var executor = Substitute.For(Substitute.For>()); + + _mockTokenProvider.GetMgGraphAccessTokenAsync( + Arg.Any(), Arg.Any>(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(Task.FromException(new HttpRequestException("Connection timeout"))); + + var graphService = new GraphApiService(_mockGraphLogger, executor, handler, _mockTokenProvider); + var service = new AgentBlueprintService(_mockLogger, graphService); + + // Act + var result = await service.DeleteAgentUserAsync("tenant-id", "user-obj-1"); + + // Assert + result.Should().BeFalse(); + } } // Simple fake handler that returns queued responses sequentially @@ -339,4 +467,14 @@ protected override Task SendAsync(HttpRequestMessage reques var resp = _responses.Dequeue(); return Task.FromResult(resp); } + + protected override void Dispose(bool disposing) + { + if (disposing) + { + while (_responses.Count > 0) + _responses.Dequeue().Dispose(); + } + base.Dispose(disposing); + } } From 674a8ac34d39993faab05247fa32a6fef4d16855 Mon Sep 17 00:00:00 2001 From: Grant Harris <96964444+gwharris7@users.noreply.github.com> Date: Tue, 24 Feb 2026 10:25:20 -0800 Subject: [PATCH 02/15] Change property name from identityParentId to agentIdentityBlueprintId --- .../Services/AgentBlueprintService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs index bdf37285..cab76d44 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs @@ -202,7 +202,7 @@ public virtual async Task> GetAgentInstancesFor foreach (var item in valueArray.EnumerateArray()) { - var parentId = item.TryGetProperty("identityParentId", out var p) ? p.GetString() : null; + var parentId = item.TryGetProperty("agentIdentityBlueprintId", out var p) ? p.GetString() : null; if (!string.Equals(parentId, blueprintId, StringComparison.OrdinalIgnoreCase)) { continue; From 77f8a67e1e9c751894a39f703486be3c7e3c60f3 Mon Sep 17 00:00:00 2001 From: Grant Harris Date: Tue, 24 Feb 2026 11:18:44 -0800 Subject: [PATCH 03/15] updated implementation to get agent user via SP id --- .../Services/AgentBlueprintService.cs | 48 +++++++++++++++++-- .../Services/AgentBlueprintServiceTests.cs | 18 +++++-- 2 files changed, 59 insertions(+), 7 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs index cab76d44..d26a0bd3 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs @@ -177,8 +177,6 @@ public virtual async Task> GetAgentInstancesFor { try { - // NOTE: "identityParentId" and "agentUserId" are placeholder property names. - // Verify the actual Graph beta API response shape before releasing. var requiredScopes = new[] { "AgentIdentityBlueprint.ReadWrite.All" }; using var doc = await _graphApiService.GraphGetAsync( tenantId, @@ -215,7 +213,7 @@ public virtual async Task> GetAgentInstancesFor } var displayName = item.TryGetProperty("displayName", out var dn) ? dn.GetString() : null; - var agentUserId = item.TryGetProperty("agentUserId", out var uid) ? uid.GetString() : null; + var agentUserId = await GetAgentUserIdForSpAsync(tenantId, spId, cancellationToken); results.Add(new AgentInstanceInfo { @@ -234,6 +232,50 @@ public virtual async Task> GetAgentInstancesFor } } + /// + /// Resolves the agentic user ID associated with a given service principal by querying + /// the /beta/users endpoint filtered by identityParentId. + /// + /// The tenant ID for authentication. + /// The service principal object ID to look up. + /// Cancellation token. + /// The user object ID if found; null otherwise. + private async Task GetAgentUserIdForSpAsync( + string tenantId, + string spId, + CancellationToken cancellationToken) + { + try + { + var requiredScopes = new[] { "AgentIdentityBlueprint.ReadWrite.All" }; + using var doc = await _graphApiService.GraphGetAsync( + tenantId, + $"/beta/users?$filter=identityParentId eq '{spId}'", + cancellationToken, + requiredScopes); + + if (doc is null) + { + return null; + } + + if (doc.RootElement.TryGetProperty("value", out var valueArray) && + valueArray.ValueKind == JsonValueKind.Array && + valueArray.GetArrayLength() > 0) + { + var firstUser = valueArray[0]; + return firstUser.TryGetProperty("id", out var idProp) ? idProp.GetString() : null; + } + + return null; + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Failed to resolve agent user ID for service principal {SpId}", spId); + return null; + } + } + /// /// Deletes an agentic user from Entra ID using the agentUsers beta endpoint. /// diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs index 01431806..64865994 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs @@ -348,20 +348,30 @@ public async Task GetAgentInstancesForBlueprintAsync_ReturnsFilteredInstances() const string blueprintId = "bp-id-123"; - // Returns two SPs; only one matches the blueprint - // NOTE: "identityParentId" and "agentUserId" are placeholder property names pending live API verification + // Response 1: GET /beta/servicePrincipals/microsoft.graph.agentIdentity + // Returns two SPs; only one matches the blueprint via agentIdentityBlueprintId handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent(JsonSerializer.Serialize(new { value = new[] { - new { id = "sp-obj-1", displayName = "Instance A", identityParentId = blueprintId, agentUserId = "user-obj-1" }, - new { id = "sp-obj-2", displayName = "Instance B", identityParentId = "other-blueprint-id", agentUserId = "user-obj-2" } + new { id = "sp-obj-1", displayName = "Instance A", agentIdentityBlueprintId = blueprintId }, + new { id = "sp-obj-2", displayName = "Instance B", agentIdentityBlueprintId = "other-blueprint-id" } } })) }); + // Response 2: GET /beta/users?$filter=identityParentId eq 'sp-obj-1' + // Secondary call to resolve the agentic user for the matching SP + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent(JsonSerializer.Serialize(new + { + value = new[] { new { id = "user-obj-1" } } + })) + }); + // Act var instances = await service.GetAgentInstancesForBlueprintAsync("tenant-id", blueprintId); From 7120cdf54ad81bec65c1b6b27a2bb34f947ad023 Mon Sep 17 00:00:00 2001 From: Grant Harris Date: Fri, 27 Feb 2026 02:30:56 -0800 Subject: [PATCH 04/15] PR review comments --- .../Commands/CleanupCommand.cs | 26 +++--- .../Services/AgentBlueprintService.cs | 76 +++++++++------- .../Commands/CleanupCommandTests.cs | 56 ++++++++++++ .../Services/AgentBlueprintServiceTests.cs | 88 ++++++++----------- 4 files changed, 150 insertions(+), 96 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs index e4e5a8c2..cc25aa57 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs @@ -13,6 +13,9 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Commands; public class CleanupCommand { + private const string AgenticUsersKey = "agentic users"; + private const string IdentitySpsKey = "identity SPs"; + public static Command CreateCommand( ILogger logger, IConfigService configService, @@ -152,8 +155,8 @@ private static Command CreateBlueprintCleanupCommand( // Delete instances first (warn and continue on failure) var failedResources = new Dictionary> { - ["agentic users"] = new List(), - ["identity SPs"] = new List() + [AgenticUsersKey] = new List(), + [IdentitySpsKey] = new List() }; foreach (var instance in instances) @@ -171,7 +174,7 @@ private static Command CreateBlueprintCleanupCommand( if (!userDeleted) { logger.LogWarning("Failed to delete agentic user {UserId} -- will continue", instance.AgentUserId); - failedResources["agentic users"].Add(instance.AgentUserId!); + failedResources[AgenticUsersKey].Add(instance.AgentUserId!); } else { @@ -190,7 +193,7 @@ private static Command CreateBlueprintCleanupCommand( if (!spDeleted) { logger.LogWarning("Failed to delete agent identity SP {SpId} -- will continue", instance.IdentitySpId); - failedResources["identity SPs"].Add(instance.IdentitySpId); + failedResources[IdentitySpsKey].Add(instance.IdentitySpId); } else { @@ -252,12 +255,9 @@ private static Command CreateBlueprintCleanupCommand( await configService.SaveStateAsync(config); logger.LogInformation("Local configuration cleared"); - // Emit orphan summary if any instance deletions failed - if (HasOrphanedResources(failedResources)) - { - PrintOrphanSummary(logger, failedResources); - } - else + // Emit orphan summary if any instance deletions failed (PrintOrphanSummary returns early if none) + PrintOrphanSummary(logger, failedResources); + if (!HasOrphanedResources(failedResources)) { logger.LogInformation(""); logger.LogInformation("Blueprint cleanup completed successfully!"); @@ -885,7 +885,7 @@ private static async Task ExecuteEndpointOnlyCleanupAsync( /// private static bool HasOrphanedResources(Dictionary> failedResources) { - return failedResources["agentic users"].Count + failedResources["identity SPs"].Count > 0; + return failedResources[AgenticUsersKey].Count + failedResources[IdentitySpsKey].Count > 0; } /// @@ -905,9 +905,9 @@ private static void PrintOrphanSummary( Console.WriteLine(); logger.LogWarning("Blueprint cleanup completed with warnings."); logger.LogWarning("The following resources could not be deleted and remain orphaned in Entra ID:"); - foreach (var userId in failedResources["agentic users"]) + foreach (var userId in failedResources[AgenticUsersKey]) logger.LogWarning(" Orphaned agentic user: {ResourceId}", userId); - foreach (var spId in failedResources["identity SPs"]) + foreach (var spId in failedResources[IdentitySpsKey]) logger.LogWarning(" Orphaned identity SP: {ResourceId}", spId); logger.LogWarning("Delete them manually via the Entra portal or Graph API."); } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs index d26a0bd3..1287339c 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs @@ -178,50 +178,60 @@ public virtual async Task> GetAgentInstancesFor try { var requiredScopes = new[] { "AgentIdentityBlueprint.ReadWrite.All" }; - using var doc = await _graphApiService.GraphGetAsync( - tenantId, - "/beta/servicePrincipals/microsoft.graph.agentIdentity", - cancellationToken, - requiredScopes); - - if (doc is null) - { - _logger.LogWarning("Failed to retrieve agent instances from Graph API"); - return Array.Empty(); - } - var results = new List(); - if (!doc.RootElement.TryGetProperty("value", out var valueArray) || - valueArray.ValueKind != JsonValueKind.Array) - { - return Array.Empty(); - } + var encodedId = Uri.EscapeDataString(blueprintId); + var firstPagePath = $"/beta/servicePrincipals/microsoft.graph.agentIdentity" + + $"?$filter=agentIdentityBlueprintId eq '{encodedId}'"; + string? nextPageUrl = null; + var isFirstPage = true; - foreach (var item in valueArray.EnumerateArray()) + do { - var parentId = item.TryGetProperty("agentIdentityBlueprintId", out var p) ? p.GetString() : null; - if (!string.Equals(parentId, blueprintId, StringComparison.OrdinalIgnoreCase)) + var requestPath = isFirstPage ? firstPagePath : nextPageUrl!; + isFirstPage = false; + + using var doc = await _graphApiService.GraphGetAsync( + tenantId, + requestPath, + cancellationToken, + requiredScopes); + + if (doc is null) { - continue; + _logger.LogWarning("Failed to retrieve agent instances from Graph API"); + return results.Count > 0 ? results : Array.Empty(); } - var spId = item.TryGetProperty("id", out var id) ? id.GetString() : null; - if (string.IsNullOrWhiteSpace(spId)) + if (doc.RootElement.TryGetProperty("value", out var valueArray) && + valueArray.ValueKind == JsonValueKind.Array) { - continue; - } + foreach (var item in valueArray.EnumerateArray()) + { + var spId = item.TryGetProperty("id", out var id) ? id.GetString() : null; + if (string.IsNullOrWhiteSpace(spId)) + { + continue; + } - var displayName = item.TryGetProperty("displayName", out var dn) ? dn.GetString() : null; - var agentUserId = await GetAgentUserIdForSpAsync(tenantId, spId, cancellationToken); + var displayName = item.TryGetProperty("displayName", out var dn) ? dn.GetString() : null; + var agentUserId = await GetAgentUserIdForSpAsync(tenantId, spId, cancellationToken); - results.Add(new AgentInstanceInfo - { - IdentitySpId = spId, - DisplayName = displayName, - AgentUserId = string.IsNullOrWhiteSpace(agentUserId) ? null : agentUserId - }); + results.Add(new AgentInstanceInfo + { + IdentitySpId = spId, + DisplayName = displayName, + AgentUserId = string.IsNullOrWhiteSpace(agentUserId) ? null : agentUserId + }); + } + } + + // Handle pagination via @odata.nextLink + nextPageUrl = doc.RootElement.TryGetProperty("@odata.nextLink", out var nextLink) + ? nextLink.GetString() + : null; } + while (!string.IsNullOrEmpty(nextPageUrl)); return results; } diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandTests.cs index 56f77cca..6d4c5c19 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandTests.cs @@ -445,6 +445,62 @@ await spyService.Received(1).DeleteAgentBlueprintAsync( Arg.Any>()); } + /// + /// Verifies that when instances are deleted successfully but the blueprint deletion fails, + /// a warning is logged about the incomplete cleanup state. + /// + [Fact] + public async Task CleanupBlueprint_WhenBlueprintDeletionFailsWithInstances_LogsWarning() + { + // Arrange + var config = CreateValidConfig(); + var expectedBlueprintId = config.AgentBlueprintId!; + _mockConfigService.LoadAsync(Arg.Any(), Arg.Any()).Returns(config); + _mockBotConfigurator.DeleteEndpointWithAgentBlueprintAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(true); + + var instances = new List + { + new() { IdentitySpId = "sp-id-1", DisplayName = "Instance A", AgentUserId = "user-id-1" } + }; + var spyService = CreateStubbedBlueprintService( + instances: instances, + deleteUserResult: true, + deleteIdentityResult: true, + deleteBlueprintResult: false); + + _mockConfirmationProvider.ConfirmAsync(Arg.Any()).Returns(true); + + var command = CleanupCommand.CreateCommand( + _mockLogger, _mockConfigService, _mockBotConfigurator, + _mockExecutor, spyService, _mockConfirmationProvider, _federatedCredentialService); + var args = new[] { "cleanup", "blueprint", "--config", "test.json" }; + + // Act + var result = await command.InvokeAsync(args); + + // Assert + result.Should().Be(0); + + await spyService.Received(1).DeleteAgentUserAsync( + config.TenantId, "user-id-1", Arg.Any()); + + await spyService.Received(1).DeleteAgentIdentityAsync( + config.TenantId, "sp-id-1", Arg.Any()); + + await spyService.Received(1).DeleteAgentBlueprintAsync( + config.TenantId, expectedBlueprintId, Arg.Any()); + + // Verify that a warning was logged about the blueprint deletion failure + _mockLogger.Received().Log( + LogLevel.Warning, + Arg.Any(), + Arg.Is(o => o.ToString()!.Contains("Blueprint deletion failed")), + Arg.Any(), + Arg.Any>()); + } + private static Agent365Config CreateValidConfig() { return new Agent365Config diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs index 64865994..ebe65957 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs @@ -328,36 +328,19 @@ public async Task DeleteAgentIdentityAsync_WhenExceptionThrown_ReturnsFalse() public async Task GetAgentInstancesForBlueprintAsync_ReturnsFilteredInstances() { // Arrange - var handler = new FakeHttpMessageHandler(); - var executor = Substitute.For(Substitute.For>()); - executor.ExecuteAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) - .Returns(callInfo => - { - var args = callInfo.ArgAt(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 = string.Empty, StandardError = string.Empty }); - }); - - _mockTokenProvider.GetMgGraphAccessTokenAsync( - Arg.Any(), Arg.Any>(), Arg.Any(), Arg.Any(), Arg.Any()) - .Returns("test-token"); - - var graphService = new GraphApiService(_mockGraphLogger, executor, handler, _mockTokenProvider); - var service = new AgentBlueprintService(_mockLogger, graphService); + var (service, handler) = CreateServiceWithFakeHandler(); - const string blueprintId = "bp-id-123"; + const string blueprintId = "a1b2c3d4-e5f6-7890-abcd-ef1234567890"; - // Response 1: GET /beta/servicePrincipals/microsoft.graph.agentIdentity - // Returns two SPs; only one matches the blueprint via agentIdentityBlueprintId + // Response 1: GET /beta/servicePrincipals/microsoft.graph.agentIdentity?$filter=agentIdentityBlueprintId eq '...' + // Server-side filtered response returns only matching SPs handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent(JsonSerializer.Serialize(new { value = new[] { - new { id = "sp-obj-1", displayName = "Instance A", agentIdentityBlueprintId = blueprintId }, - new { id = "sp-obj-2", displayName = "Instance B", agentIdentityBlueprintId = "other-blueprint-id" } + new { id = "sp-obj-1", displayName = "Instance A", agentIdentityBlueprintId = blueprintId } } })) }); @@ -380,29 +363,15 @@ public async Task GetAgentInstancesForBlueprintAsync_ReturnsFilteredInstances() instances[0].IdentitySpId.Should().Be("sp-obj-1"); instances[0].DisplayName.Should().Be("Instance A"); instances[0].AgentUserId.Should().Be("user-obj-1"); + + handler.Dispose(); } [Fact] public async Task GetAgentInstancesForBlueprintAsync_ReturnsEmpty_WhenNoneFound() { // Arrange - var handler = new FakeHttpMessageHandler(); - var executor = Substitute.For(Substitute.For>()); - executor.ExecuteAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) - .Returns(callInfo => - { - var args = callInfo.ArgAt(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 = string.Empty, StandardError = string.Empty }); - }); - - _mockTokenProvider.GetMgGraphAccessTokenAsync( - Arg.Any(), Arg.Any>(), Arg.Any(), Arg.Any(), Arg.Any()) - .Returns("test-token"); - - var graphService = new GraphApiService(_mockGraphLogger, executor, handler, _mockTokenProvider); - var service = new AgentBlueprintService(_mockLogger, graphService); + var (service, handler) = CreateServiceWithFakeHandler(); handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) { @@ -410,25 +379,19 @@ public async Task GetAgentInstancesForBlueprintAsync_ReturnsEmpty_WhenNoneFound( }); // Act - var instances = await service.GetAgentInstancesForBlueprintAsync("tenant-id", "bp-id-123"); + var instances = await service.GetAgentInstancesForBlueprintAsync("tenant-id", "b2c3d4e5-f6a7-8901-bcde-f12345678901"); // Assert instances.Should().BeEmpty(); + + handler.Dispose(); } [Fact] public async Task DeleteAgentUserAsync_ReturnsTrue_OnSuccess() { // Arrange - var handler = new FakeHttpMessageHandler(); - var executor = Substitute.For(Substitute.For>()); - - _mockTokenProvider.GetMgGraphAccessTokenAsync( - Arg.Any(), Arg.Any>(), Arg.Any(), Arg.Any(), Arg.Any()) - .Returns("test-token"); - - var graphService = new GraphApiService(_mockGraphLogger, executor, handler, _mockTokenProvider); - var service = new AgentBlueprintService(_mockLogger, graphService); + var (service, handler) = CreateServiceWithFakeHandler(); // Queue HTTP response for DELETE /beta/agentUsers/{userId} handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.NoContent)); @@ -438,6 +401,8 @@ public async Task DeleteAgentUserAsync_ReturnsTrue_OnSuccess() // Assert result.Should().BeTrue(); + + handler.Dispose(); } [Fact] @@ -445,12 +410,17 @@ public async Task DeleteAgentUserAsync_ReturnsFalse_OnGraphError() { // Arrange var handler = new FakeHttpMessageHandler(); - var executor = Substitute.For(Substitute.For>()); _mockTokenProvider.GetMgGraphAccessTokenAsync( Arg.Any(), Arg.Any>(), Arg.Any(), Arg.Any(), Arg.Any()) .Returns(Task.FromException(new HttpRequestException("Connection timeout"))); + var executor = Substitute.For(Substitute.For>()); + executor.ExecuteAsync(Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(new CommandResult + { ExitCode = 0, StandardOutput = string.Empty, StandardError = string.Empty })); + var graphService = new GraphApiService(_mockGraphLogger, executor, handler, _mockTokenProvider); var service = new AgentBlueprintService(_mockLogger, graphService); @@ -459,6 +429,24 @@ public async Task DeleteAgentUserAsync_ReturnsFalse_OnGraphError() // Assert result.Should().BeFalse(); + + handler.Dispose(); + } + + private (AgentBlueprintService service, FakeHttpMessageHandler handler) CreateServiceWithFakeHandler() + { + var handler = new FakeHttpMessageHandler(); + var executor = Substitute.For(Substitute.For>()); + executor.ExecuteAsync(Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(new CommandResult + { ExitCode = 0, StandardOutput = string.Empty, StandardError = string.Empty })); + _mockTokenProvider.GetMgGraphAccessTokenAsync( + Arg.Any(), Arg.Any>(), Arg.Any(), + Arg.Any(), Arg.Any()) + .Returns("test-token"); + var graphService = new GraphApiService(_mockGraphLogger, executor, handler, _mockTokenProvider); + return (new AgentBlueprintService(_mockLogger, graphService), handler); } } From 5813cb6db107a8c66a957edd5ae8bc79bb3e4d61 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Fri, 27 Feb 2026 09:47:01 -0800 Subject: [PATCH 05/15] Guarantee FakeHttpMessageHandler disposal in AgentBlueprintServiceTests (#297) * Initial plan * Fix FakeHttpMessageHandler disposal to use using statements Co-authored-by: gwharris7 <96964444+gwharris7@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: gwharris7 <96964444+gwharris7@users.noreply.github.com> --- .../Services/AgentBlueprintServiceTests.cs | 100 +++++++++--------- 1 file changed, 49 insertions(+), 51 deletions(-) diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs index ebe65957..09496587 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs @@ -329,42 +329,42 @@ public async Task GetAgentInstancesForBlueprintAsync_ReturnsFilteredInstances() { // Arrange var (service, handler) = CreateServiceWithFakeHandler(); - - const string blueprintId = "a1b2c3d4-e5f6-7890-abcd-ef1234567890"; - - // Response 1: GET /beta/servicePrincipals/microsoft.graph.agentIdentity?$filter=agentIdentityBlueprintId eq '...' - // Server-side filtered response returns only matching SPs - handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) + using (handler) { - Content = new StringContent(JsonSerializer.Serialize(new + const string blueprintId = "a1b2c3d4-e5f6-7890-abcd-ef1234567890"; + + // Response 1: GET /beta/servicePrincipals/microsoft.graph.agentIdentity?$filter=agentIdentityBlueprintId eq '...' + // Server-side filtered response returns only matching SPs + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) { - value = new[] + Content = new StringContent(JsonSerializer.Serialize(new { - new { id = "sp-obj-1", displayName = "Instance A", agentIdentityBlueprintId = blueprintId } - } - })) - }); + value = new[] + { + new { id = "sp-obj-1", displayName = "Instance A", agentIdentityBlueprintId = blueprintId } + } + })) + }); - // Response 2: GET /beta/users?$filter=identityParentId eq 'sp-obj-1' - // Secondary call to resolve the agentic user for the matching SP - handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) - { - Content = new StringContent(JsonSerializer.Serialize(new + // Response 2: GET /beta/users?$filter=identityParentId eq 'sp-obj-1' + // Secondary call to resolve the agentic user for the matching SP + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) { - value = new[] { new { id = "user-obj-1" } } - })) - }); - - // Act - var instances = await service.GetAgentInstancesForBlueprintAsync("tenant-id", blueprintId); + Content = new StringContent(JsonSerializer.Serialize(new + { + value = new[] { new { id = "user-obj-1" } } + })) + }); - // Assert - instances.Should().HaveCount(1); - instances[0].IdentitySpId.Should().Be("sp-obj-1"); - instances[0].DisplayName.Should().Be("Instance A"); - instances[0].AgentUserId.Should().Be("user-obj-1"); + // Act + var instances = await service.GetAgentInstancesForBlueprintAsync("tenant-id", blueprintId); - handler.Dispose(); + // Assert + instances.Should().HaveCount(1); + instances[0].IdentitySpId.Should().Be("sp-obj-1"); + instances[0].DisplayName.Should().Be("Instance A"); + instances[0].AgentUserId.Should().Be("user-obj-1"); + } } [Fact] @@ -372,19 +372,19 @@ public async Task GetAgentInstancesForBlueprintAsync_ReturnsEmpty_WhenNoneFound( { // Arrange var (service, handler) = CreateServiceWithFakeHandler(); - - handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) + using (handler) { - Content = new StringContent(JsonSerializer.Serialize(new { value = Array.Empty() })) - }); + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent(JsonSerializer.Serialize(new { value = Array.Empty() })) + }); - // Act - var instances = await service.GetAgentInstancesForBlueprintAsync("tenant-id", "b2c3d4e5-f6a7-8901-bcde-f12345678901"); + // Act + var instances = await service.GetAgentInstancesForBlueprintAsync("tenant-id", "b2c3d4e5-f6a7-8901-bcde-f12345678901"); - // Assert - instances.Should().BeEmpty(); - - handler.Dispose(); + // Assert + instances.Should().BeEmpty(); + } } [Fact] @@ -392,24 +392,24 @@ public async Task DeleteAgentUserAsync_ReturnsTrue_OnSuccess() { // Arrange var (service, handler) = CreateServiceWithFakeHandler(); + using (handler) + { + // Queue HTTP response for DELETE /beta/agentUsers/{userId} + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.NoContent)); - // Queue HTTP response for DELETE /beta/agentUsers/{userId} - handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.NoContent)); - - // Act - var result = await service.DeleteAgentUserAsync("tenant-id", "user-obj-1"); + // Act + var result = await service.DeleteAgentUserAsync("tenant-id", "user-obj-1"); - // Assert - result.Should().BeTrue(); - - handler.Dispose(); + // Assert + result.Should().BeTrue(); + } } [Fact] public async Task DeleteAgentUserAsync_ReturnsFalse_OnGraphError() { // Arrange - var handler = new FakeHttpMessageHandler(); + using var handler = new FakeHttpMessageHandler(); _mockTokenProvider.GetMgGraphAccessTokenAsync( Arg.Any(), Arg.Any>(), Arg.Any(), Arg.Any(), Arg.Any()) @@ -429,8 +429,6 @@ public async Task DeleteAgentUserAsync_ReturnsFalse_OnGraphError() // Assert result.Should().BeFalse(); - - handler.Dispose(); } private (AgentBlueprintService service, FakeHttpMessageHandler handler) CreateServiceWithFakeHandler() From 9b57f0862dc290ab8a1d14b6f8e6585a996dc448 Mon Sep 17 00:00:00 2001 From: Grant Harris Date: Fri, 27 Feb 2026 11:30:59 -0800 Subject: [PATCH 06/15] server-side filtering for agent users --- .../Services/AgentBlueprintService.cs | 130 +++++++++--------- .../Services/AgentBlueprintServiceTests.cs | 13 +- 2 files changed, 75 insertions(+), 68 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs index 1287339c..350c8835 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs @@ -178,60 +178,56 @@ public virtual async Task> GetAgentInstancesFor try { var requiredScopes = new[] { "AgentIdentityBlueprint.ReadWrite.All" }; - var results = new List(); - var encodedId = Uri.EscapeDataString(blueprintId); - var firstPagePath = $"/beta/servicePrincipals/microsoft.graph.agentIdentity" + - $"?$filter=agentIdentityBlueprintId eq '{encodedId}'"; - string? nextPageUrl = null; - var isFirstPage = true; - do - { - var requestPath = isFirstPage ? firstPagePath : nextPageUrl!; - isFirstPage = false; + // Fetch agent identity SPs and agent users for this blueprint in parallel (2 calls total) + var spTask = FetchAllPagesAsync( + tenantId, + $"/beta/servicePrincipals/microsoft.graph.agentIdentity?$filter=agentIdentityBlueprintId eq '{encodedId}'", + requiredScopes, + cancellationToken); - using var doc = await _graphApiService.GraphGetAsync( - tenantId, - requestPath, - cancellationToken, - requiredScopes); + var userTask = FetchAllPagesAsync( + tenantId, + $"/beta/users/microsoft.graph.agentUser?$filter=agentIdentityBlueprintId eq '{encodedId}'", + requiredScopes, + cancellationToken); + + var spItems = await spTask; + var userItems = await userTask; - if (doc is null) + // Build lookup: identityParentId (SP object ID) -> user object ID + var userBySpId = new Dictionary(StringComparer.OrdinalIgnoreCase); + foreach (var user in userItems) + { + var parentId = user.TryGetProperty("identityParentId", out var p) ? p.GetString() : null; + var userId = user.TryGetProperty("id", out var uid) ? uid.GetString() : null; + if (!string.IsNullOrWhiteSpace(parentId) && !string.IsNullOrWhiteSpace(userId)) { - _logger.LogWarning("Failed to retrieve agent instances from Graph API"); - return results.Count > 0 ? results : Array.Empty(); + userBySpId[parentId] = userId; } + } - if (doc.RootElement.TryGetProperty("value", out var valueArray) && - valueArray.ValueKind == JsonValueKind.Array) + // Correlate SPs with their agent users + var results = new List(); + foreach (var item in spItems) + { + var spId = item.TryGetProperty("id", out var id) ? id.GetString() : null; + if (string.IsNullOrWhiteSpace(spId)) { - foreach (var item in valueArray.EnumerateArray()) - { - var spId = item.TryGetProperty("id", out var id) ? id.GetString() : null; - if (string.IsNullOrWhiteSpace(spId)) - { - continue; - } - - var displayName = item.TryGetProperty("displayName", out var dn) ? dn.GetString() : null; - var agentUserId = await GetAgentUserIdForSpAsync(tenantId, spId, cancellationToken); - - results.Add(new AgentInstanceInfo - { - IdentitySpId = spId, - DisplayName = displayName, - AgentUserId = string.IsNullOrWhiteSpace(agentUserId) ? null : agentUserId - }); - } + continue; } - // Handle pagination via @odata.nextLink - nextPageUrl = doc.RootElement.TryGetProperty("@odata.nextLink", out var nextLink) - ? nextLink.GetString() - : null; + var displayName = item.TryGetProperty("displayName", out var dn) ? dn.GetString() : null; + userBySpId.TryGetValue(spId, out var agentUserId); + + results.Add(new AgentInstanceInfo + { + IdentitySpId = spId, + DisplayName = displayName, + AgentUserId = string.IsNullOrWhiteSpace(agentUserId) ? null : agentUserId + }); } - while (!string.IsNullOrEmpty(nextPageUrl)); return results; } @@ -243,47 +239,51 @@ public virtual async Task> GetAgentInstancesFor } /// - /// Resolves the agentic user ID associated with a given service principal by querying - /// the /beta/users endpoint filtered by identityParentId. + /// Fetches all pages of a Graph API collection, following @odata.nextLink pagination. + /// Returns the deserialized "value" array items from all pages. /// - /// The tenant ID for authentication. - /// The service principal object ID to look up. - /// Cancellation token. - /// The user object ID if found; null otherwise. - private async Task GetAgentUserIdForSpAsync( + private async Task> FetchAllPagesAsync( string tenantId, - string spId, + string initialPath, + string[] requiredScopes, CancellationToken cancellationToken) { - try + var items = new List(); + string? nextPageUrl = null; + var isFirstPage = true; + + do { - var requiredScopes = new[] { "AgentIdentityBlueprint.ReadWrite.All" }; + var requestPath = isFirstPage ? initialPath : nextPageUrl!; + isFirstPage = false; + using var doc = await _graphApiService.GraphGetAsync( tenantId, - $"/beta/users?$filter=identityParentId eq '{spId}'", + requestPath, cancellationToken, requiredScopes); if (doc is null) { - return null; + break; } if (doc.RootElement.TryGetProperty("value", out var valueArray) && - valueArray.ValueKind == JsonValueKind.Array && - valueArray.GetArrayLength() > 0) + valueArray.ValueKind == JsonValueKind.Array) { - var firstUser = valueArray[0]; - return firstUser.TryGetProperty("id", out var idProp) ? idProp.GetString() : null; + foreach (var item in valueArray.EnumerateArray()) + { + items.Add(item.Clone()); + } } - return null; - } - catch (Exception ex) - { - _logger.LogWarning(ex, "Failed to resolve agent user ID for service principal {SpId}", spId); - return null; + nextPageUrl = doc.RootElement.TryGetProperty("@odata.nextLink", out var nextLink) + ? nextLink.GetString() + : null; } + while (!string.IsNullOrEmpty(nextPageUrl)); + + return items; } /// diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs index ebe65957..b8b7471b 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs @@ -345,13 +345,13 @@ public async Task GetAgentInstancesForBlueprintAsync_ReturnsFilteredInstances() })) }); - // Response 2: GET /beta/users?$filter=identityParentId eq 'sp-obj-1' - // Secondary call to resolve the agentic user for the matching SP + // Response 2: GET /beta/users/microsoft.graph.agentUser?$filter=agentIdentityBlueprintId eq '...' + // Bulk query returns all agent users for the blueprint; correlated via identityParentId handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent(JsonSerializer.Serialize(new { - value = new[] { new { id = "user-obj-1" } } + value = new[] { new { id = "user-obj-1", identityParentId = "sp-obj-1" } } })) }); @@ -373,6 +373,13 @@ public async Task GetAgentInstancesForBlueprintAsync_ReturnsEmpty_WhenNoneFound( // Arrange var (service, handler) = CreateServiceWithFakeHandler(); + // Response 1: SPs query returns empty + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent(JsonSerializer.Serialize(new { value = Array.Empty() })) + }); + + // Response 2: Users query returns empty (both run in parallel) handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent(JsonSerializer.Serialize(new { value = Array.Empty() })) From 22fc347caed77e9607d2ac315292377c6226b6e3 Mon Sep 17 00:00:00 2001 From: Grant Harris Date: Fri, 27 Feb 2026 11:31:11 -0800 Subject: [PATCH 07/15] helper method for tests --- .../Services/AgentBlueprintServiceTests.cs | 54 ++++++------------- 1 file changed, 16 insertions(+), 38 deletions(-) diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs index b8b7471b..03e6c912 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs @@ -166,13 +166,12 @@ public async Task SetInheritablePermissionsAsync_Patches_WhenPresent() public async Task DeleteAgentIdentityAsync_WithValidIdentity_ReturnsTrue() { // Arrange - var handler = new FakeHttpMessageHandler(); - var graphService = new GraphApiService(_mockGraphLogger, _mockExecutor, handler, _mockTokenProvider); - var service = new AgentBlueprintService(_mockLogger, graphService); + var (service, handler) = CreateServiceWithFakeHandler(); const string tenantId = "12345678-1234-1234-1234-123456789012"; const string identityId = "identity-sp-id-123"; + // Override with specific scope assertion _mockTokenProvider.GetMgGraphAccessTokenAsync( tenantId, Arg.Is>(scopes => scopes.Contains("AgentIdentityBlueprint.ReadWrite.All")), @@ -195,27 +194,19 @@ await _mockTokenProvider.Received(1).GetMgGraphAccessTokenAsync( false, Arg.Any(), Arg.Any()); + + handler.Dispose(); } [Fact] public async Task DeleteAgentIdentityAsync_WhenResourceNotFound_ReturnsTrueIdempotent() { // Arrange - var handler = new FakeHttpMessageHandler(); - var graphService = new GraphApiService(_mockGraphLogger, _mockExecutor, handler, _mockTokenProvider); - var service = new AgentBlueprintService(_mockLogger, graphService); + var (service, handler) = CreateServiceWithFakeHandler(); const string tenantId = "12345678-1234-1234-1234-123456789012"; const string identityId = "non-existent-identity"; - _mockTokenProvider.GetMgGraphAccessTokenAsync( - Arg.Any(), - Arg.Any>(), - Arg.Any(), - Arg.Any(), - Arg.Any()) - .Returns("fake-token"); - handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.NotFound) { Content = new StringContent("{\"error\": {\"code\": \"Request_ResourceNotFound\"}}") @@ -226,6 +217,8 @@ public async Task DeleteAgentIdentityAsync_WhenResourceNotFound_ReturnsTrueIdemp // Assert result.Should().BeTrue("404 should be treated as success for idempotent deletion"); + + handler.Dispose(); } [Fact] @@ -257,21 +250,11 @@ public async Task DeleteAgentIdentityAsync_WhenTokenProviderIsNull_ReturnsFalse( public async Task DeleteAgentIdentityAsync_WhenDeletionFails_ReturnsFalse() { // Arrange - var handler = new FakeHttpMessageHandler(); - var graphService = new GraphApiService(_mockGraphLogger, _mockExecutor, handler, _mockTokenProvider); - var service = new AgentBlueprintService(_mockLogger, graphService); + var (service, handler) = CreateServiceWithFakeHandler(); const string tenantId = "12345678-1234-1234-1234-123456789012"; const string identityId = "identity-123"; - _mockTokenProvider.GetMgGraphAccessTokenAsync( - Arg.Any(), - Arg.Any>(), - Arg.Any(), - Arg.Any(), - Arg.Any()) - .Returns("fake-token"); - handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.Forbidden) { Content = new StringContent("{\"error\": {\"code\": \"Authorization_RequestDenied\"}}") @@ -289,19 +272,20 @@ public async Task DeleteAgentIdentityAsync_WhenDeletionFails_ReturnsFalse() Arg.Is(o => o.ToString()!.Contains("Graph DELETE") && o.ToString()!.Contains("403")), Arg.Any(), Arg.Any>()); + + handler.Dispose(); } [Fact] public async Task DeleteAgentIdentityAsync_WhenExceptionThrown_ReturnsFalse() { // Arrange - var handler = new FakeHttpMessageHandler(); - var graphService = new GraphApiService(_mockGraphLogger, _mockExecutor, handler, _mockTokenProvider); - var service = new AgentBlueprintService(_mockLogger, graphService); + var (service, handler) = CreateServiceWithFakeHandler(); const string tenantId = "12345678-1234-1234-1234-123456789012"; const string identityId = "identity-123"; + // Override token provider to throw _mockTokenProvider.GetMgGraphAccessTokenAsync( Arg.Any(), Arg.Any>(), @@ -322,6 +306,8 @@ public async Task DeleteAgentIdentityAsync_WhenExceptionThrown_ReturnsFalse() Arg.Is(o => o.ToString()!.Contains("Exception deleting agent identity")), Arg.Any(), Arg.Any>()); + + handler.Dispose(); } [Fact] @@ -416,21 +402,13 @@ public async Task DeleteAgentUserAsync_ReturnsTrue_OnSuccess() public async Task DeleteAgentUserAsync_ReturnsFalse_OnGraphError() { // Arrange - var handler = new FakeHttpMessageHandler(); + var (service, handler) = CreateServiceWithFakeHandler(); + // Override token provider to throw _mockTokenProvider.GetMgGraphAccessTokenAsync( Arg.Any(), Arg.Any>(), Arg.Any(), Arg.Any(), Arg.Any()) .Returns(Task.FromException(new HttpRequestException("Connection timeout"))); - var executor = Substitute.For(Substitute.For>()); - executor.ExecuteAsync(Arg.Any(), Arg.Any(), Arg.Any(), - Arg.Any(), Arg.Any(), Arg.Any()) - .Returns(Task.FromResult(new CommandResult - { ExitCode = 0, StandardOutput = string.Empty, StandardError = string.Empty })); - - var graphService = new GraphApiService(_mockGraphLogger, executor, handler, _mockTokenProvider); - var service = new AgentBlueprintService(_mockLogger, graphService); - // Act var result = await service.DeleteAgentUserAsync("tenant-id", "user-obj-1"); From ff0ddc9ce7887fcdfd83b9471110c809ad4719e1 Mon Sep 17 00:00:00 2001 From: Grant Harris <96964444+gwharris7@users.noreply.github.com> Date: Fri, 27 Feb 2026 11:49:18 -0800 Subject: [PATCH 08/15] Update src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../Services/AgentBlueprintService.cs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs index 350c8835..4ebd1cae 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs @@ -180,22 +180,18 @@ public virtual async Task> GetAgentInstancesFor var requiredScopes = new[] { "AgentIdentityBlueprint.ReadWrite.All" }; var encodedId = Uri.EscapeDataString(blueprintId); - // Fetch agent identity SPs and agent users for this blueprint in parallel (2 calls total) - var spTask = FetchAllPagesAsync( + // Fetch agent identity SPs and agent users for this blueprint sequentially to avoid races on shared HTTP headers + var spItems = await FetchAllPagesAsync( tenantId, $"/beta/servicePrincipals/microsoft.graph.agentIdentity?$filter=agentIdentityBlueprintId eq '{encodedId}'", requiredScopes, cancellationToken); - var userTask = FetchAllPagesAsync( + var userItems = await FetchAllPagesAsync( tenantId, $"/beta/users/microsoft.graph.agentUser?$filter=agentIdentityBlueprintId eq '{encodedId}'", requiredScopes, cancellationToken); - - var spItems = await spTask; - var userItems = await userTask; - // Build lookup: identityParentId (SP object ID) -> user object ID var userBySpId = new Dictionary(StringComparer.OrdinalIgnoreCase); foreach (var user in userItems) From d16e107cd1ab834123e837772bb86b4e6a16fea9 Mon Sep 17 00:00:00 2001 From: Grant Harris <96964444+gwharris7@users.noreply.github.com> Date: Fri, 27 Feb 2026 11:55:51 -0800 Subject: [PATCH 09/15] Update src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../Services/AgentBlueprintService.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs index 4ebd1cae..76205421 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs @@ -261,7 +261,14 @@ private async Task> FetchAllPagesAsync( if (doc is null) { - break; + _logger.LogError( + "Failed to retrieve data from Microsoft Graph for tenant '{TenantId}' and request path '{RequestPath}'. " + + "GraphGetAsync returned null, which likely indicates a non-success response or authentication issue.", + tenantId, + requestPath); + + throw new InvalidOperationException( + "Failed to retrieve data from Microsoft Graph. See logs for details about the underlying request failure."); } if (doc.RootElement.TryGetProperty("value", out var valueArray) && From 6a54122ac507d64945845a9dd13c0c1534a628c0 Mon Sep 17 00:00:00 2001 From: Grant Harris <96964444+gwharris7@users.noreply.github.com> Date: Fri, 27 Feb 2026 13:48:01 -0800 Subject: [PATCH 10/15] Update src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../Services/AgentBlueprintService.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs index 76205421..4de39fcd 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs @@ -183,13 +183,13 @@ public virtual async Task> GetAgentInstancesFor // Fetch agent identity SPs and agent users for this blueprint sequentially to avoid races on shared HTTP headers var spItems = await FetchAllPagesAsync( tenantId, - $"/beta/servicePrincipals/microsoft.graph.agentIdentity?$filter=agentIdentityBlueprintId eq '{encodedId}'", + $"/beta/servicePrincipals/microsoft.graph.agentIdentity?$filter=agentIdentityBlueprintId eq '{encodedId}'&$select=id,displayName", requiredScopes, cancellationToken); var userItems = await FetchAllPagesAsync( tenantId, - $"/beta/users/microsoft.graph.agentUser?$filter=agentIdentityBlueprintId eq '{encodedId}'", + $"/beta/users/microsoft.graph.agentUser?$filter=agentIdentityBlueprintId eq '{encodedId}'&$select=id,identityParentId", requiredScopes, cancellationToken); // Build lookup: identityParentId (SP object ID) -> user object ID From 5f95f8449d0c0faf4898ecbdd8022c29f7a91395 Mon Sep 17 00:00:00 2001 From: Grant Harris <96964444+gwharris7@users.noreply.github.com> Date: Mon, 2 Mar 2026 16:50:02 -0800 Subject: [PATCH 11/15] Update src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../Commands/CleanupCommand.cs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs index cc25aa57..064f0cf1 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs @@ -121,9 +121,18 @@ private static Command CreateBlueprintCleanupCommand( // Query for agent instances linked to this blueprint before showing preview logger.LogInformation("Querying for agent instances linked to blueprint..."); - var instances = await agentBlueprintService.GetAgentInstancesForBlueprintAsync( - config.TenantId, - config.AgentBlueprintId); + List instances; + try + { + instances = (await agentBlueprintService.GetAgentInstancesForBlueprintAsync( + config.TenantId, + config.AgentBlueprintId))?.ToList() ?? new List(); + } + catch (Exception ex) + { + logger.LogError(ex, "Failed to query agent instances for blueprint {BlueprintId}. Aborting cleanup.", config.AgentBlueprintId); + return; + } // Show preview logger.LogInformation(""); From 6de710d7b6ca97a4e56bb0050b453bfd930763d4 Mon Sep 17 00:00:00 2001 From: Grant Harris <96964444+gwharris7@users.noreply.github.com> Date: Mon, 2 Mar 2026 16:50:37 -0800 Subject: [PATCH 12/15] Update src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../Commands/CleanupCommand.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs index 064f0cf1..fe662b66 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs @@ -912,7 +912,7 @@ private static void PrintOrphanSummary( } Console.WriteLine(); - logger.LogWarning("Blueprint cleanup completed with warnings."); + logger.LogWarning("Blueprint cleanup encountered warnings."); logger.LogWarning("The following resources could not be deleted and remain orphaned in Entra ID:"); foreach (var userId in failedResources[AgenticUsersKey]) logger.LogWarning(" Orphaned agentic user: {ResourceId}", userId); From 9adf6dc43219878d7aa3b73ffcd7ee11c20a2fbb Mon Sep 17 00:00:00 2001 From: Grant Harris <96964444+gwharris7@users.noreply.github.com> Date: Mon, 2 Mar 2026 16:54:50 -0800 Subject: [PATCH 13/15] Update src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../Commands/CleanupCommand.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs index fe662b66..2a953e0b 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs @@ -121,12 +121,12 @@ private static Command CreateBlueprintCleanupCommand( // Query for agent instances linked to this blueprint before showing preview logger.LogInformation("Querying for agent instances linked to blueprint..."); - List instances; + List instances; try { instances = (await agentBlueprintService.GetAgentInstancesForBlueprintAsync( config.TenantId, - config.AgentBlueprintId))?.ToList() ?? new List(); + config.AgentBlueprintId))?.ToList() ?? new List(); } catch (Exception ex) { From 9a446e4d7ca58404d1503dbbe2fa0b7be015f4b2 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Mon, 2 Mar 2026 17:04:42 -0800 Subject: [PATCH 14/15] Surface query failure explicitly in GetAgentInstancesForBlueprintAsync (#305) * Initial plan * Surface query failure explicitly in GetAgentInstancesForBlueprintAsync Co-authored-by: gwharris7 <96964444+gwharris7@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: gwharris7 <96964444+gwharris7@users.noreply.github.com> --- .../Services/AgentBlueprintService.cs | 92 +++++++++---------- .../Services/AgentBlueprintServiceTests.cs | 16 ++++ 2 files changed, 59 insertions(+), 49 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs index 4de39fcd..a5f8acb2 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs @@ -164,74 +164,68 @@ public virtual async Task DeleteAgentIdentityAsync( /// /// Queries Entra ID for all agent identity service principals linked to the given blueprint. - /// Returns an empty list if the query fails or no instances are found. + /// Returns an empty list when no instances are found. + /// Throws if the query fails so callers can distinguish a true "no instances" result from a query error. /// /// The tenant ID for authentication. /// The blueprint application ID or object ID. /// Cancellation token. /// List of agent instances linked to the blueprint. + /// Thrown when the Graph query fails. public virtual async Task> GetAgentInstancesForBlueprintAsync( string tenantId, string blueprintId, CancellationToken cancellationToken = default) { - try - { - var requiredScopes = new[] { "AgentIdentityBlueprint.ReadWrite.All" }; - var encodedId = Uri.EscapeDataString(blueprintId); + var requiredScopes = new[] { "AgentIdentityBlueprint.ReadWrite.All" }; + var encodedId = Uri.EscapeDataString(blueprintId); - // Fetch agent identity SPs and agent users for this blueprint sequentially to avoid races on shared HTTP headers - var spItems = await FetchAllPagesAsync( - tenantId, - $"/beta/servicePrincipals/microsoft.graph.agentIdentity?$filter=agentIdentityBlueprintId eq '{encodedId}'&$select=id,displayName", - requiredScopes, - cancellationToken); + // Fetch agent identity SPs and agent users for this blueprint sequentially to avoid races on shared HTTP headers + var spItems = await FetchAllPagesAsync( + tenantId, + $"/beta/servicePrincipals/microsoft.graph.agentIdentity?$filter=agentIdentityBlueprintId eq '{encodedId}'&$select=id,displayName", + requiredScopes, + cancellationToken); - var userItems = await FetchAllPagesAsync( - tenantId, - $"/beta/users/microsoft.graph.agentUser?$filter=agentIdentityBlueprintId eq '{encodedId}'&$select=id,identityParentId", - requiredScopes, - cancellationToken); - // Build lookup: identityParentId (SP object ID) -> user object ID - var userBySpId = new Dictionary(StringComparer.OrdinalIgnoreCase); - foreach (var user in userItems) + var userItems = await FetchAllPagesAsync( + tenantId, + $"/beta/users/microsoft.graph.agentUser?$filter=agentIdentityBlueprintId eq '{encodedId}'&$select=id,identityParentId", + requiredScopes, + cancellationToken); + // Build lookup: identityParentId (SP object ID) -> user object ID + var userBySpId = new Dictionary(StringComparer.OrdinalIgnoreCase); + foreach (var user in userItems) + { + var parentId = user.TryGetProperty("identityParentId", out var p) ? p.GetString() : null; + var userId = user.TryGetProperty("id", out var uid) ? uid.GetString() : null; + if (!string.IsNullOrWhiteSpace(parentId) && !string.IsNullOrWhiteSpace(userId)) { - var parentId = user.TryGetProperty("identityParentId", out var p) ? p.GetString() : null; - var userId = user.TryGetProperty("id", out var uid) ? uid.GetString() : null; - if (!string.IsNullOrWhiteSpace(parentId) && !string.IsNullOrWhiteSpace(userId)) - { - userBySpId[parentId] = userId; - } + userBySpId[parentId] = userId; } + } - // Correlate SPs with their agent users - var results = new List(); - foreach (var item in spItems) + // Correlate SPs with their agent users + var results = new List(); + foreach (var item in spItems) + { + var spId = item.TryGetProperty("id", out var id) ? id.GetString() : null; + if (string.IsNullOrWhiteSpace(spId)) { - var spId = item.TryGetProperty("id", out var id) ? id.GetString() : null; - if (string.IsNullOrWhiteSpace(spId)) - { - continue; - } - - var displayName = item.TryGetProperty("displayName", out var dn) ? dn.GetString() : null; - userBySpId.TryGetValue(spId, out var agentUserId); - - results.Add(new AgentInstanceInfo - { - IdentitySpId = spId, - DisplayName = displayName, - AgentUserId = string.IsNullOrWhiteSpace(agentUserId) ? null : agentUserId - }); + continue; } - return results; - } - catch (Exception ex) - { - _logger.LogError(ex, "Exception querying agent instances for blueprint {BlueprintId}", blueprintId); - return Array.Empty(); + var displayName = item.TryGetProperty("displayName", out var dn) ? dn.GetString() : null; + userBySpId.TryGetValue(spId, out var agentUserId); + + results.Add(new AgentInstanceInfo + { + IdentitySpId = spId, + DisplayName = displayName, + AgentUserId = string.IsNullOrWhiteSpace(agentUserId) ? null : agentUserId + }); } + + return results; } /// diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs index 449a4291..5d617683 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs @@ -380,6 +380,22 @@ public async Task GetAgentInstancesForBlueprintAsync_ReturnsEmpty_WhenNoneFound( } } + [Fact] + public async Task GetAgentInstancesForBlueprintAsync_Throws_WhenGraphQueryFails() + { + // Arrange + var (service, _) = CreateServiceWithFakeHandler(); + + // Override token provider to throw so the Graph call fails + _mockTokenProvider.GetMgGraphAccessTokenAsync( + Arg.Any(), Arg.Any>(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(Task.FromException(new HttpRequestException("Connection timeout"))); + + // Act & Assert - exception must propagate so callers can abort rather than proceeding with 0 instances + await service.Invoking(s => s.GetAgentInstancesForBlueprintAsync("tenant-id", "blueprint-id")) + .Should().ThrowAsync(); + } + [Fact] public async Task DeleteAgentUserAsync_ReturnsTrue_OnSuccess() { From 8800b54f43fbaf7e228b13d791ac2a270508741b Mon Sep 17 00:00:00 2001 From: Grant Harris Date: Tue, 3 Mar 2026 18:23:04 -0800 Subject: [PATCH 15/15] feedback from PR comments --- .../Commands/CleanupCommand.cs | 9 ++++--- .../Services/GraphApiService.cs | 25 +++++++++++++------ .../Commands/CleanupCommandTests.cs | 10 +++++++- .../Services/AgentBlueprintServiceTests.cs | 12 ++++++++- 4 files changed, 43 insertions(+), 13 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs index cc25aa57..dffbb110 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs @@ -232,9 +232,13 @@ private static Command CreateBlueprintCleanupCommand( if (!deleted) { - Console.WriteLine(); - logger.LogWarning("Blueprint deletion failed."); + logger.LogWarning(""); + logger.LogWarning("Blueprint deletion failed. The blueprint still exists in Entra ID."); PrintOrphanSummary(logger, failedResources); + if (!HasOrphanedResources(failedResources)) + { + logger.LogWarning("All agent instances were deleted. Retry 'a365 cleanup blueprint' or delete the blueprint manually via the Entra portal or Graph API."); + } return; } @@ -902,7 +906,6 @@ private static void PrintOrphanSummary( return; } - Console.WriteLine(); logger.LogWarning("Blueprint cleanup completed with warnings."); logger.LogWarning("The following resources could not be deleted and remain orphaned in Entra ID:"); foreach (var userId in failedResources[AgenticUsersKey]) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs index 1854f999..7cc8dab6 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs @@ -284,8 +284,13 @@ private async Task EnsureGraphHeadersAsync(string tenantId, CancellationTo var url = relativePath.StartsWith("http", StringComparison.OrdinalIgnoreCase) ? relativePath : $"https://graph.microsoft.com{relativePath}"; - var resp = await _httpClient.GetAsync(url, ct); - if (!resp.IsSuccessStatusCode) return null; + using var resp = await _httpClient.GetAsync(url, ct); + if (!resp.IsSuccessStatusCode) + { + var errorBody = await resp.Content.ReadAsStringAsync(ct); + _logger.LogError("Graph GET {Url} failed {Code} {Reason}: {Body}", url, (int)resp.StatusCode, resp.ReasonPhrase, errorBody); + return null; + } var json = await resp.Content.ReadAsStringAsync(ct); return JsonDocument.Parse(json); @@ -298,9 +303,13 @@ private async Task EnsureGraphHeadersAsync(string tenantId, CancellationTo ? relativePath : $"https://graph.microsoft.com{relativePath}"; var content = new StringContent(JsonSerializer.Serialize(payload), Encoding.UTF8, "application/json"); - var resp = await _httpClient.PostAsync(url, content, ct); + using var resp = await _httpClient.PostAsync(url, content, ct); var body = await resp.Content.ReadAsStringAsync(ct); - if (!resp.IsSuccessStatusCode) return null; + if (!resp.IsSuccessStatusCode) + { + _logger.LogError("Graph POST {Url} failed {Code} {Reason}: {Body}", url, (int)resp.StatusCode, resp.ReasonPhrase, body); + return null; + } return string.IsNullOrWhiteSpace(body) ? null : JsonDocument.Parse(body); } @@ -320,7 +329,7 @@ public virtual async Task GraphPostWithResponseAsync(string tenan : $"https://graph.microsoft.com{relativePath}"; var content = new StringContent(JsonSerializer.Serialize(payload), Encoding.UTF8, "application/json"); - var resp = await _httpClient.PostAsync(url, content, ct); + using var resp = await _httpClient.PostAsync(url, content, ct); var body = await resp.Content.ReadAsStringAsync(ct); JsonDocument? json = null; @@ -350,8 +359,8 @@ public virtual async Task GraphPatchAsync(string tenantId, string relative ? relativePath : $"https://graph.microsoft.com{relativePath}"; var content = new StringContent(JsonSerializer.Serialize(payload), Encoding.UTF8, "application/json"); - var request = new HttpRequestMessage(new HttpMethod("PATCH"), url) { Content = content }; - var resp = await _httpClient.SendAsync(request, ct); + using var request = new HttpRequestMessage(new HttpMethod("PATCH"), url) { Content = content }; + using var resp = await _httpClient.SendAsync(request, ct); // Many PATCH calls return 204 NoContent on success if (!resp.IsSuccessStatusCode) @@ -359,7 +368,7 @@ public virtual async Task GraphPatchAsync(string tenantId, string relative var body = await resp.Content.ReadAsStringAsync(ct); _logger.LogError("Graph PATCH {Url} failed {Code} {Reason}: {Body}", url, (int)resp.StatusCode, resp.ReasonPhrase, body); } - + return resp.IsSuccessStatusCode; } diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandTests.cs index 6d4c5c19..9b5b0ebe 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandTests.cs @@ -496,7 +496,15 @@ await spyService.Received(1).DeleteAgentBlueprintAsync( _mockLogger.Received().Log( LogLevel.Warning, Arg.Any(), - Arg.Is(o => o.ToString()!.Contains("Blueprint deletion failed")), + Arg.Is(o => o.ToString()!.Contains("Blueprint deletion failed. The blueprint still exists in Entra ID.")), + Arg.Any(), + Arg.Any>()); + + // Verify that the retry guidance message is also logged + _mockLogger.Received().Log( + LogLevel.Warning, + Arg.Any(), + Arg.Is(o => o.ToString()!.Contains("All agent instances were deleted. Retry 'a365 cleanup blueprint'")), Arg.Any(), Arg.Any>()); } diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs index 449a4291..2cf6e626 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs @@ -439,15 +439,21 @@ public async Task DeleteAgentUserAsync_ReturnsFalse_OnGraphError() internal class FakeHttpMessageHandler : HttpMessageHandler { private readonly Queue _responses = new(); + private readonly List _sentResponses = new(); public void QueueResponse(HttpResponseMessage resp) => _responses.Enqueue(resp); protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) { if (_responses.Count == 0) - return Task.FromResult(new HttpResponseMessage(HttpStatusCode.NotFound) { Content = new StringContent("") }); + { + var fallback = new HttpResponseMessage(HttpStatusCode.NotFound) { Content = new StringContent("") }; + _sentResponses.Add(fallback); + return Task.FromResult(fallback); + } var resp = _responses.Dequeue(); + _sentResponses.Add(resp); return Task.FromResult(resp); } @@ -455,6 +461,10 @@ protected override void Dispose(bool disposing) { if (disposing) { + foreach (var resp in _sentResponses) + resp.Dispose(); + _sentResponses.Clear(); + while (_responses.Count > 0) _responses.Dequeue().Dispose(); }