From 641cbd2fb1042c3065720cd3e662dc078022b9d8 Mon Sep 17 00:00:00 2001 From: Bhaarath Raguru Date: Wed, 29 Apr 2026 16:03:47 -0700 Subject: [PATCH 01/14] feat: add vscode.dev and localhost redirect URIs to public clients app Co-Authored-By: Claude Opus 4.6 --- .../Commands/RegisterCommandExecutor.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs index 59cf8d16..9289693b 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs @@ -617,7 +617,7 @@ private void DisplayRegistrationSummary(ResolvedInput input) var copilotRedirectUri = $"ms-appx-web://Microsoft.AAD.BrokerPlugin/{publicClientsClientId}"; try { - await _graphApiService.UpdateAppPublicClientRedirectUrisAsync(tenantId, publicClientsObjectId, new[] { copilotRedirectUri, "http://localhost:8080/callback" }); + await _graphApiService.UpdateAppPublicClientRedirectUrisAsync(tenantId, publicClientsObjectId, new[] { copilotRedirectUri, "http://localhost:8080/callback", "https://vscode.dev/redirect", "http://localhost" }); _logger.LogDebug("Set redirect URI on '{AppName}' ({ObjectId}): {Uri}", publicClientsAppName, publicClientsObjectId, copilotRedirectUri); } catch (Exception ex) From e85a421b73d58fdec41f4fd1008ac3cc9a4a42c8 Mon Sep 17 00:00:00 2001 From: Bhaarath Raguru Date: Wed, 29 Apr 2026 16:06:36 -0700 Subject: [PATCH 02/14] fix: remove unused --config option from register-external-mcp-server command Co-Authored-By: Claude Opus 4.6 --- .../Commands/DevelopMcpCommand.cs | 3 --- .../Commands/DevelopMcpCommandTests.cs | 18 ------------------ 2 files changed, 21 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs index b5d7f916..3695ff7e 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs @@ -850,9 +850,6 @@ private static Command CreateRegisterExternalMcpServerSubcommand( var serviceTreeIdOption = new Option("--service-tree-id", description: "ServiceTree ID for Entra app registration (required in Microsoft corporate tenants)"); command.AddOption(serviceTreeIdOption); - var configOption = new Option(["-c", "--config"], getDefaultValue: () => "a365.config.json", description: "Configuration file path"); - command.AddOption(configOption); - var publisherOption = new Option("--publisher", description: "Publisher name (required, used in MOS package metadata)"); command.AddOption(publisherOption); diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandTests.cs index 7980cd6a..a78c190e 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandTests.cs @@ -249,20 +249,6 @@ public void AllSubcommands_SupportDryRunOption() } } - [Theory] - [InlineData("register-external-mcp-server")] - public void ConfigDependentSubcommands_SupportConfigOption(string subcommandName) - { - // Act - var command = DevelopMcpCommand.CreateCommand(_mockLogger, _mockToolingService); - var subcommand = command.Subcommands.First(sc => sc.Name == subcommandName); - - // Assert - var configOption = subcommand.Options.FirstOrDefault(o => o.Name == "config"); - configOption.Should().NotBeNull($"Subcommand '{subcommandName}' should have --config option"); - configOption!.Aliases.Should().Contain("-c", $"Config option should have -c alias in '{subcommandName}'"); - } - [Fact] public void RegisterExternalMcpServerSubcommand_HasAllExpectedOptions() { @@ -291,7 +277,6 @@ public void RegisterExternalMcpServerSubcommand_HasAllExpectedOptions() optionNames.Should().Contain("remote-scopes"); optionNames.Should().Contain("tenant-id"); optionNames.Should().Contain("service-tree-id"); - optionNames.Should().Contain("config"); optionNames.Should().Contain("publisher"); optionNames.Should().Contain("description"); optionNames.Should().Contain("dry-run"); @@ -314,9 +299,6 @@ public void RegisterExternalMcpServerSubcommand_HasAllExpectedOptions() var tenantIdOption = options.First(o => o.Name == "tenant-id"); tenantIdOption.Aliases.Should().Contain("-t"); - var configOption = options.First(o => o.Name == "config"); - configOption.Aliases.Should().Contain("-c"); - var verboseOption = options.First(o => o.Name == "verbose"); verboseOption.Aliases.Should().Contain("-v"); } From d5f488e3e553f416c9b355a4320ee512f475a073 Mon Sep 17 00:00:00 2001 From: Bhaarath Raguru Date: Wed, 29 Apr 2026 18:59:30 -0700 Subject: [PATCH 03/14] fix: show friendly error for duplicate server name registration Detect database constraint violation from the API and display a clean user-facing message instead of raw error details. Raw details are still available with --verbose. Also demote ValidateResponseAsync body-level errors to debug so callers control the user-facing message. Co-Authored-By: Claude Opus 4.6 --- .../Commands/RegisterCommandExecutor.cs | 18 +++++++++++++++--- .../Services/Agent365ToolingService.cs | 4 ++-- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs index 9289693b..f4cfd2c3 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs @@ -143,15 +143,27 @@ internal async Task ExecuteAsync(RawRegisterArgs args) { _logger.LogError("Failed to register MCP server '{ServerName}': {Error}", input.ServerName, ex.Message); _logger.LogDebug("Exception details: {Exception}", ex.ToString()); - _logger.LogWarning("Entra app registrations were NOT rolled back. Delete them manually in the Azure portal if needed."); + _logger.LogWarning("Entra app registrations were NOT rolled back. Use 'cleanup-external-mcp-server-resources' to remove them if needed."); return; } if (addResponse is null || !addResponse.IsSuccess) { var errorMsg = addResponse?.Message ?? "No response received"; - _logger.LogError("Failed to add MCP server {ServerName}: {Error}", input.ServerName, errorMsg); - _logger.LogWarning("Entra app registrations were NOT rolled back. Delete them manually in the Azure portal if needed."); + + if (errorMsg.Contains("violates a database constraint", StringComparison.OrdinalIgnoreCase) + || errorMsg.Contains("delete the existing record", StringComparison.OrdinalIgnoreCase)) + { + Console.WriteLine(); + Console.WriteLine($"ERROR: A server named '{input.ServerName}' already exists. Please choose a different name."); + _logger.LogDebug("Raw server error: {Error}", errorMsg); + } + else + { + _logger.LogError("Failed to add MCP server {ServerName}: {Error}", input.ServerName, errorMsg); + } + + Console.WriteLine($"Entra app registrations were NOT rolled back. Use 'cleanup-external-mcp-server-resources' to remove them if needed."); return; } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Agent365ToolingService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Agent365ToolingService.cs index 462b6606..9d82cea1 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Agent365ToolingService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Agent365ToolingService.cs @@ -97,10 +97,10 @@ public Agent365ToolingService( errorMessage += $" - {statusResponse.Error}"; } - _logger.LogError("{Operation} failed: {Message}", operationName, errorMessage); + _logger.LogDebug("{Operation} failed: {Message}", operationName, errorMessage); if (!string.IsNullOrWhiteSpace(serverCorrelationId)) { - _logger.LogError("Server correlation ID (x-ms-correlation-id): {CorrelationId}", serverCorrelationId); + _logger.LogDebug("Server correlation ID (x-ms-correlation-id): {CorrelationId}", serverCorrelationId); } return (false, responseContent); } From 765b891f4c5271a28a4e74c3f4016a857ccd5d98 Mon Sep 17 00:00:00 2001 From: Bhaarath Raguru Date: Wed, 29 Apr 2026 19:05:52 -0700 Subject: [PATCH 04/14] fix: show duplicate server name error in red Co-Authored-By: Claude Opus 4.6 --- .../Commands/RegisterCommandExecutor.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs index f4cfd2c3..643245fc 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs @@ -155,7 +155,10 @@ internal async Task ExecuteAsync(RawRegisterArgs args) || errorMsg.Contains("delete the existing record", StringComparison.OrdinalIgnoreCase)) { Console.WriteLine(); + var prevColor = Console.ForegroundColor; + Console.ForegroundColor = ConsoleColor.Red; Console.WriteLine($"ERROR: A server named '{input.ServerName}' already exists. Please choose a different name."); + Console.ForegroundColor = prevColor; _logger.LogDebug("Raw server error: {Error}", errorMsg); } else From 421044a820c672973aa0c5a0e38e14cbd6882bb9 Mon Sep 17 00:00:00 2001 From: Bhaarath Raguru Date: Wed, 29 Apr 2026 19:14:30 -0700 Subject: [PATCH 05/14] fix: remove reference to deleted cleanup command in error messages Co-Authored-By: Claude Opus 4.6 --- .../Commands/RegisterCommandExecutor.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs index 643245fc..b9330540 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs @@ -143,7 +143,7 @@ internal async Task ExecuteAsync(RawRegisterArgs args) { _logger.LogError("Failed to register MCP server '{ServerName}': {Error}", input.ServerName, ex.Message); _logger.LogDebug("Exception details: {Exception}", ex.ToString()); - _logger.LogWarning("Entra app registrations were NOT rolled back. Use 'cleanup-external-mcp-server-resources' to remove them if needed."); + _logger.LogWarning("Entra app registrations were NOT rolled back. Delete them manually in the Azure portal if needed."); return; } @@ -166,7 +166,7 @@ internal async Task ExecuteAsync(RawRegisterArgs args) _logger.LogError("Failed to add MCP server {ServerName}: {Error}", input.ServerName, errorMsg); } - Console.WriteLine($"Entra app registrations were NOT rolled back. Use 'cleanup-external-mcp-server-resources' to remove them if needed."); + Console.WriteLine($"Entra app registrations were NOT rolled back. Delete them manually in the Azure portal if needed."); return; } From 0e25c0a7f95ae45b5c532dc0d6c7d0a512f3d7af Mon Sep 17 00:00:00 2001 From: Bhaarath Raguru Date: Wed, 29 Apr 2026 19:24:36 -0700 Subject: [PATCH 06/14] fix: address PR review - add config negative assertion, log duplicate error, correlation ID at warning Co-Authored-By: Claude Opus 4.6 --- .../Commands/RegisterCommandExecutor.cs | 1 + .../Services/Agent365ToolingService.cs | 2 +- .../Commands/DevelopMcpCommandTests.cs | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs index b9330540..1e7f0403 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs @@ -159,6 +159,7 @@ internal async Task ExecuteAsync(RawRegisterArgs args) Console.ForegroundColor = ConsoleColor.Red; Console.WriteLine($"ERROR: A server named '{input.ServerName}' already exists. Please choose a different name."); Console.ForegroundColor = prevColor; + _logger.LogError("A server named '{ServerName}' already exists. Please choose a different name.", input.ServerName); _logger.LogDebug("Raw server error: {Error}", errorMsg); } else diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Agent365ToolingService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Agent365ToolingService.cs index 9d82cea1..374357e9 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Agent365ToolingService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Agent365ToolingService.cs @@ -100,7 +100,7 @@ public Agent365ToolingService( _logger.LogDebug("{Operation} failed: {Message}", operationName, errorMessage); if (!string.IsNullOrWhiteSpace(serverCorrelationId)) { - _logger.LogDebug("Server correlation ID (x-ms-correlation-id): {CorrelationId}", serverCorrelationId); + _logger.LogWarning("Server correlation ID (x-ms-correlation-id): {CorrelationId}", serverCorrelationId); } return (false, responseContent); } diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandTests.cs index a78c190e..9e1d2416 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandTests.cs @@ -281,6 +281,7 @@ public void RegisterExternalMcpServerSubcommand_HasAllExpectedOptions() optionNames.Should().Contain("description"); optionNames.Should().Contain("dry-run"); optionNames.Should().Contain("verbose"); + optionNames.Should().NotContain("config"); // Verify critical aliases var serverNameOption = options.First(o => o.Name == "server-name"); From 32376a00c5a72634d7fde22efd5f5aea46caac6f Mon Sep 17 00:00:00 2001 From: Bhaarath Raguru Date: Wed, 29 Apr 2026 19:31:29 -0700 Subject: [PATCH 07/14] fix: also log rollback warning so it appears in file logs Co-Authored-By: Claude Opus 4.6 --- .../Commands/RegisterCommandExecutor.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs index 1e7f0403..dacfc8a1 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs @@ -167,6 +167,7 @@ internal async Task ExecuteAsync(RawRegisterArgs args) _logger.LogError("Failed to add MCP server {ServerName}: {Error}", input.ServerName, errorMsg); } + _logger.LogWarning("Entra app registrations were NOT rolled back. Delete them manually in the Azure portal if needed."); Console.WriteLine($"Entra app registrations were NOT rolled back. Delete them manually in the Azure portal if needed."); return; } From 6d8f0433106f31eefe82154d51d2d457e3ff9ab5 Mon Sep 17 00:00:00 2001 From: Bhaarath Raguru Date: Thu, 30 Apr 2026 11:00:00 -0700 Subject: [PATCH 08/14] fix: add retry with exponential backoff for Graph API calls and escalate failures to errors All post-registration Graph calls (redirect URIs, API permissions) now retry up to 3 times with exponential backoff (2s, 4s, 8s) to handle transient failures like throttling, replication lag, and network blips. Failures are now logged at Error level instead of Warning to prevent silent misconfiguration of Entra apps. Co-Authored-By: Claude Opus 4.6 --- .../Commands/RegisterCommandExecutor.cs | 110 ++++++++++++++---- 1 file changed, 87 insertions(+), 23 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs index dacfc8a1..c0b143c2 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs @@ -4,6 +4,7 @@ using Microsoft.Agents.A365.DevTools.Cli.Helpers; using Microsoft.Agents.A365.DevTools.Cli.Models; using Microsoft.Agents.A365.DevTools.Cli.Services; +using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; using Microsoft.Extensions.Logging; using System.Text.Json; @@ -40,6 +41,7 @@ internal class RegisterCommandExecutor private readonly ILogger _logger; private readonly IAgent365ToolingService _toolingService; private readonly GraphApiService? _graphApiService; + private readonly RetryHelper _retryHelper; internal RegisterCommandExecutor( ILogger logger, @@ -49,6 +51,7 @@ internal RegisterCommandExecutor( _logger = logger; _toolingService = toolingService; _graphApiService = graphApiService; + _retryHelper = new RetryHelper(logger, maxRetries: 3, baseDelaySeconds: 2); } private sealed record ResolvedInput @@ -632,15 +635,27 @@ private void DisplayRegistrationSummary(ResolvedInput input) _logger.LogInformation("Created Entra app '{AppName}' (clientId: {ClientId})", publicClientsAppName, publicClientsClientId); var copilotRedirectUri = $"ms-appx-web://Microsoft.AAD.BrokerPlugin/{publicClientsClientId}"; + var publicClientUris = new[] { copilotRedirectUri, "http://localhost:8080/callback", "https://vscode.dev/redirect", "http://localhost" }; try { - await _graphApiService.UpdateAppPublicClientRedirectUrisAsync(tenantId, publicClientsObjectId, new[] { copilotRedirectUri, "http://localhost:8080/callback", "https://vscode.dev/redirect", "http://localhost" }); - _logger.LogDebug("Set redirect URI on '{AppName}' ({ObjectId}): {Uri}", publicClientsAppName, publicClientsObjectId, copilotRedirectUri); + var success = await _retryHelper.ExecuteWithRetryAsync( + async ct => await _graphApiService.UpdateAppPublicClientRedirectUrisAsync(tenantId, publicClientsObjectId, publicClientUris, ct), + result => !result); + if (!success) + { + var msg = $"Failed to set redirect URIs on Public Clients app '{publicClientsAppName}' after retries."; + _logger.LogError(msg); + warnings.Add(msg); + } + else + { + _logger.LogDebug("Set redirect URI on '{AppName}' ({ObjectId}): {Uri}", publicClientsAppName, publicClientsObjectId, copilotRedirectUri); + } } catch (Exception ex) { - var msg = $"Failed to set redirect URI on Public Clients app: {ex.Message}"; - _logger.LogWarning(msg); + var msg = $"Failed to set redirect URIs on Public Clients app: {ex.Message}"; + _logger.LogError(msg); warnings.Add(msg); } } @@ -743,13 +758,24 @@ private async Task ConfigureRedirectUrisAsync( var a365NonTcUri = DevelopMcpCommand.RemoveTcPrefix(a365RedirectUri); var a365Uris = DevelopMcpCommand.BuildRedirectUriList(a365RedirectUri, a365TcUri, a365NonTcUri); _logger.LogDebug("Updating redirect URIs on '{AppName}' ({ObjectId}): {RedirectUris}", apps.A365AppName, apps.A365AppObjectId, string.Join(", ", a365Uris)); - await _graphApiService!.UpdateAppRedirectUrisAsync(tenantId, apps.A365AppObjectId, a365Uris); - _logger.LogInformation("Updated redirect URIs on '{AppName}'", apps.A365AppName); + var success = await _retryHelper.ExecuteWithRetryAsync( + async ct => await _graphApiService!.UpdateAppRedirectUrisAsync(tenantId, apps.A365AppObjectId, a365Uris, ct), + result => !result); + if (!success) + { + var msg = $"Failed to update redirect URIs on A365 Proxy app '{apps.A365AppName}' after retries."; + _logger.LogError(msg); + warnings.Add(msg); + } + else + { + _logger.LogInformation("Updated redirect URIs on '{AppName}'", apps.A365AppName); + } } catch (Exception ex) { var msg = $"Failed to update redirect URIs on A365 Proxy app: {ex.Message}"; - _logger.LogWarning(msg); + _logger.LogError(msg); warnings.Add(msg); } } @@ -768,13 +794,24 @@ private async Task ConfigureRedirectUrisAsync( var remoteNonTcUri = DevelopMcpCommand.RemoveTcPrefix(remoteRedirectUri); var remoteUris = DevelopMcpCommand.BuildRedirectUriList(remoteRedirectUri, remoteTcUri, remoteNonTcUri); _logger.LogDebug("Updating redirect URIs on '{AppName}' ({ObjectId}): {RedirectUris}", apps.RemoteProxyAppName, apps.RemoteProxyObjectId, string.Join(", ", remoteUris)); - await _graphApiService!.UpdateAppRedirectUrisAsync(tenantId, apps.RemoteProxyObjectId, remoteUris); - _logger.LogInformation("Updated redirect URIs on '{AppName}'", apps.RemoteProxyAppName); + var success = await _retryHelper.ExecuteWithRetryAsync( + async ct => await _graphApiService!.UpdateAppRedirectUrisAsync(tenantId, apps.RemoteProxyObjectId, remoteUris, ct), + result => !result); + if (!success) + { + var msg = $"Failed to update redirect URIs on Remote Proxy app '{apps.RemoteProxyAppName}' after retries."; + _logger.LogError(msg); + warnings.Add(msg); + } + else + { + _logger.LogInformation("Updated redirect URIs on '{AppName}'", apps.RemoteProxyAppName); + } } catch (Exception ex) { var msg = $"Failed to update redirect URIs on Remote Proxy app: {ex.Message}"; - _logger.LogWarning(msg); + _logger.LogError(msg); warnings.Add(msg); } } @@ -816,13 +853,21 @@ private async Task ConfigureApiPermissionsAsync( if (remoteScopeId.HasValue) { _logger.LogDebug("Adding API permission '{ScopeName}' (resource: {ResourceAppId}) on '{AppName}' ({ObjectId})", scopeName, resourceAppId, apps.RemoteProxyAppName, apps.RemoteProxyObjectId); - await _graphApiService.AddRequiredResourceAccessAsync( - tenantId, apps.RemoteProxyObjectId, resourceAppId, remoteScopeId.Value); + var success = await _retryHelper.ExecuteWithRetryAsync( + async ct => await _graphApiService.AddRequiredResourceAccessAsync( + tenantId, apps.RemoteProxyObjectId, resourceAppId, remoteScopeId.Value, ct), + result => !result); + if (!success) + { + var msg = $"Failed to add API permission on RemoteProxy app '{apps.RemoteProxyAppName}' after retries."; + _logger.LogError(msg); + warnings.Add(msg); + } } else { var msg = $"Could not find scope '{scopeName}' on resource app {resourceAppId}. API permission not added to RemoteProxy app."; - _logger.LogWarning(msg); + _logger.LogError(msg); warnings.Add(msg); } } @@ -836,7 +881,7 @@ await _graphApiService.AddRequiredResourceAccessAsync( catch (Exception ex) { var msg = $"Failed to add API permissions on RemoteProxy app: {ex.Message}"; - _logger.LogWarning(msg); + _logger.LogError(msg); warnings.Add(msg); } } @@ -853,14 +898,25 @@ await _graphApiService.AddRequiredResourceAccessAsync( try { _logger.LogDebug("Adding PPMI 'Tools.ListInvoke.All' permission (resource: {PpmiAppId}) on '{AppName}' ({ObjectId})", ppmiAppClientId, apps.A365AppName, apps.A365AppObjectId); - await _graphApiService.AddRequiredResourceAccessAsync( - tenantId, apps.A365AppObjectId, ppmiAppClientId, ppmiScopeId.Value); - _logger.LogInformation("Added API permission on '{AppName}'", apps.A365AppName); + var a365Success = await _retryHelper.ExecuteWithRetryAsync( + async ct => await _graphApiService.AddRequiredResourceAccessAsync( + tenantId, apps.A365AppObjectId, ppmiAppClientId, ppmiScopeId.Value, ct), + result => !result); + if (!a365Success) + { + var msg = $"Failed to add PPMI permission on A365 Proxy app '{apps.A365AppName}' after retries."; + _logger.LogError(msg); + warnings.Add(msg); + } + else + { + _logger.LogInformation("Added API permission on '{AppName}'", apps.A365AppName); + } } catch (Exception ex) { var msg = $"Failed to add PPMI permission on A365 Proxy app: {ex.Message}"; - _logger.LogWarning(msg); + _logger.LogError(msg); warnings.Add(msg); } @@ -869,13 +925,21 @@ await _graphApiService.AddRequiredResourceAccessAsync( try { _logger.LogDebug("Adding PPMI 'Tools.ListInvoke.All' permission (resource: {PpmiAppId}) on '{AppName}' ({ObjectId})", ppmiAppClientId, apps.PublicClientsAppName, apps.PublicClientsObjectId); - await _graphApiService.AddRequiredResourceAccessAsync( - tenantId, apps.PublicClientsObjectId, ppmiAppClientId, ppmiScopeId.Value); + var copilotSuccess = await _retryHelper.ExecuteWithRetryAsync( + async ct => await _graphApiService.AddRequiredResourceAccessAsync( + tenantId, apps.PublicClientsObjectId, ppmiAppClientId, ppmiScopeId.Value, ct), + result => !result); + if (!copilotSuccess) + { + var msg = $"Failed to add PPMI permission on Public Clients app '{apps.PublicClientsAppName}' after retries."; + _logger.LogError(msg); + warnings.Add(msg); + } } catch (Exception ex) { - var msg = $"Failed to add PPMI permission on Copilot app: {ex.Message}"; - _logger.LogWarning(msg); + var msg = $"Failed to add PPMI permission on Public Clients app: {ex.Message}"; + _logger.LogError(msg); warnings.Add(msg); } } @@ -883,7 +947,7 @@ await _graphApiService.AddRequiredResourceAccessAsync( else { var msg = $"Could not find 'Tools.ListInvoke.All' scope on PPMI app {ppmiAppClientId}. API permissions not added."; - _logger.LogWarning(msg); + _logger.LogError(msg); warnings.Add(msg); } } From 44a77e7c8b06ff9a5ab0fda81c21d5bb1f00417b Mon Sep 17 00:00:00 2001 From: Bhaarath Raguru Date: Thu, 30 Apr 2026 11:34:28 -0700 Subject: [PATCH 09/14] feat: parallelize independent Graph API calls using Task.WhenAll Post-registration Entra app configuration (redirect URIs + API permissions) now runs all independent Graph calls concurrently via Task.WhenAll with ConcurrentBag for thread-safe warning collection. Retry increased to 5 attempts with 3s base delay (max 93s) to match Entra replication patterns. Co-Authored-By: Claude Opus 4.6 --- .../Commands/RegisterCommandExecutor.cs | 252 ++++++++++-------- 1 file changed, 135 insertions(+), 117 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs index c0b143c2..3f3b28fa 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs @@ -51,7 +51,7 @@ internal RegisterCommandExecutor( _logger = logger; _toolingService = toolingService; _graphApiService = graphApiService; - _retryHelper = new RetryHelper(logger, maxRetries: 3, baseDelaySeconds: 2); + _retryHelper = new RetryHelper(logger, maxRetries: 5, baseDelaySeconds: 3); } private sealed record ResolvedInput @@ -177,8 +177,7 @@ internal async Task ExecuteAsync(RawRegisterArgs args) _logger.LogDebug("Successfully added MCP server {ServerName}", input.ServerName); - await ConfigureRedirectUrisAsync(input, apps, addResponse, tenantId, warnings); - await ConfigureApiPermissionsAsync(input, apps, addResponse, tenantId, warnings); + await ConfigureEntraAppsAsync(input, apps, addResponse, tenantId, warnings); DisplayResults(input, addResponse.Server?.RemoteMCPServerProxyRedirectUri, warnings); } @@ -743,170 +742,181 @@ private static AddMcpServerRequest BuildRequest(ResolvedInput input, EntraAppSet }; } - private async Task ConfigureRedirectUrisAsync( + private async Task ConfigureEntraAppsAsync( ResolvedInput input, EntraAppSet apps, AddMcpServerResponse response, string tenantId, List warnings) { + var tasks = new List(); + var concurrentWarnings = new System.Collections.Concurrent.ConcurrentBag(); + var a365RedirectUri = response.Server?.A365ProxyRedirectUri; var remoteRedirectUri = response.Server?.RemoteMCPServerProxyRedirectUri; if (!string.IsNullOrWhiteSpace(a365RedirectUri)) { - try + tasks.Add(Task.Run(async () => { - var a365TcUri = DevelopMcpCommand.AddTcPrefix(a365RedirectUri); - var a365NonTcUri = DevelopMcpCommand.RemoveTcPrefix(a365RedirectUri); - var a365Uris = DevelopMcpCommand.BuildRedirectUriList(a365RedirectUri, a365TcUri, a365NonTcUri); - _logger.LogDebug("Updating redirect URIs on '{AppName}' ({ObjectId}): {RedirectUris}", apps.A365AppName, apps.A365AppObjectId, string.Join(", ", a365Uris)); - var success = await _retryHelper.ExecuteWithRetryAsync( - async ct => await _graphApiService!.UpdateAppRedirectUrisAsync(tenantId, apps.A365AppObjectId, a365Uris, ct), - result => !result); - if (!success) + try { - var msg = $"Failed to update redirect URIs on A365 Proxy app '{apps.A365AppName}' after retries."; - _logger.LogError(msg); - warnings.Add(msg); + var a365TcUri = DevelopMcpCommand.AddTcPrefix(a365RedirectUri); + var a365NonTcUri = DevelopMcpCommand.RemoveTcPrefix(a365RedirectUri); + var a365Uris = DevelopMcpCommand.BuildRedirectUriList(a365RedirectUri, a365TcUri, a365NonTcUri); + _logger.LogDebug("Updating redirect URIs on '{AppName}' ({ObjectId})", apps.A365AppName, apps.A365AppObjectId); + var success = await _retryHelper.ExecuteWithRetryAsync( + async ct => await _graphApiService!.UpdateAppRedirectUrisAsync(tenantId, apps.A365AppObjectId, a365Uris, ct), + result => !result); + if (!success) + { + var msg = $"Failed to update redirect URIs on A365 Proxy app '{apps.A365AppName}' after retries."; + _logger.LogError(msg); + concurrentWarnings.Add(msg); + } + else + { + _logger.LogInformation("Updated redirect URIs on '{AppName}'", apps.A365AppName); + } } - else + catch (Exception ex) { - _logger.LogInformation("Updated redirect URIs on '{AppName}'", apps.A365AppName); + var msg = $"Failed to update redirect URIs on A365 Proxy app: {ex.Message}"; + _logger.LogError(msg); + concurrentWarnings.Add(msg); } - } - catch (Exception ex) - { - var msg = $"Failed to update redirect URIs on A365 Proxy app: {ex.Message}"; - _logger.LogError(msg); - warnings.Add(msg); - } + })); } else { var msg = "A365 Proxy redirect URI was not returned by the server. Redirect URI configuration skipped."; _logger.LogWarning(msg); - warnings.Add(msg); + concurrentWarnings.Add(msg); } if (input.IsEntra && !string.IsNullOrWhiteSpace(remoteRedirectUri) && apps.RemoteProxyObjectId != null) { - try + tasks.Add(Task.Run(async () => { - var remoteTcUri = DevelopMcpCommand.AddTcPrefix(remoteRedirectUri); - var remoteNonTcUri = DevelopMcpCommand.RemoveTcPrefix(remoteRedirectUri); - var remoteUris = DevelopMcpCommand.BuildRedirectUriList(remoteRedirectUri, remoteTcUri, remoteNonTcUri); - _logger.LogDebug("Updating redirect URIs on '{AppName}' ({ObjectId}): {RedirectUris}", apps.RemoteProxyAppName, apps.RemoteProxyObjectId, string.Join(", ", remoteUris)); - var success = await _retryHelper.ExecuteWithRetryAsync( - async ct => await _graphApiService!.UpdateAppRedirectUrisAsync(tenantId, apps.RemoteProxyObjectId, remoteUris, ct), - result => !result); - if (!success) + try { - var msg = $"Failed to update redirect URIs on Remote Proxy app '{apps.RemoteProxyAppName}' after retries."; - _logger.LogError(msg); - warnings.Add(msg); + var remoteTcUri = DevelopMcpCommand.AddTcPrefix(remoteRedirectUri); + var remoteNonTcUri = DevelopMcpCommand.RemoveTcPrefix(remoteRedirectUri); + var remoteUris = DevelopMcpCommand.BuildRedirectUriList(remoteRedirectUri, remoteTcUri, remoteNonTcUri); + _logger.LogDebug("Updating redirect URIs on '{AppName}' ({ObjectId})", apps.RemoteProxyAppName, apps.RemoteProxyObjectId); + var success = await _retryHelper.ExecuteWithRetryAsync( + async ct => await _graphApiService!.UpdateAppRedirectUrisAsync(tenantId, apps.RemoteProxyObjectId, remoteUris, ct), + result => !result); + if (!success) + { + var msg = $"Failed to update redirect URIs on Remote Proxy app '{apps.RemoteProxyAppName}' after retries."; + _logger.LogError(msg); + concurrentWarnings.Add(msg); + } + else + { + _logger.LogInformation("Updated redirect URIs on '{AppName}'", apps.RemoteProxyAppName); + } } - else + catch (Exception ex) { - _logger.LogInformation("Updated redirect URIs on '{AppName}'", apps.RemoteProxyAppName); + var msg = $"Failed to update redirect URIs on Remote Proxy app: {ex.Message}"; + _logger.LogError(msg); + concurrentWarnings.Add(msg); } - } - catch (Exception ex) - { - var msg = $"Failed to update redirect URIs on Remote Proxy app: {ex.Message}"; - _logger.LogError(msg); - warnings.Add(msg); - } + })); } else if (input.IsEntra) { var msg = "Remote MCP Proxy redirect URI was not returned by the server. Redirect URI configuration skipped."; _logger.LogWarning(msg); - warnings.Add(msg); + concurrentWarnings.Add(msg); } - } - private async Task ConfigureApiPermissionsAsync( - ResolvedInput input, EntraAppSet apps, AddMcpServerResponse response, - string tenantId, List warnings) - { if (input.IsEntra && apps.RemoteProxyObjectId != null && !string.IsNullOrWhiteSpace(input.RemoteScopes)) { - try + tasks.Add(Task.Run(async () => { - var scopeUri = input.RemoteScopes.Trim(); - string? resourceAppId = null; - string? scopeName = null; - - if (scopeUri.StartsWith("api://", StringComparison.OrdinalIgnoreCase)) + try { - var path = scopeUri.Substring("api://".Length); - var slashIndex = path.IndexOf('/'); - if (slashIndex > 0) + var scopeUri = input.RemoteScopes.Trim(); + string? resourceAppId = null; + string? scopeName = null; + + if (scopeUri.StartsWith("api://", StringComparison.OrdinalIgnoreCase)) { - resourceAppId = path.Substring(0, slashIndex); - scopeName = path.Substring(slashIndex + 1); + var path = scopeUri.Substring("api://".Length); + var slashIndex = path.IndexOf('/'); + if (slashIndex > 0) + { + resourceAppId = path.Substring(0, slashIndex); + scopeName = path.Substring(slashIndex + 1); + } } - } - if (!string.IsNullOrWhiteSpace(resourceAppId) && !string.IsNullOrWhiteSpace(scopeName)) - { - _logger.LogDebug("Looking up scope '{ScopeName}' on resource app {ResourceAppId}...", scopeName, resourceAppId); - var remoteScopeId = await _graphApiService!.GetOAuth2PermissionScopeIdAsync(tenantId, resourceAppId, scopeName); - if (remoteScopeId.HasValue) + if (!string.IsNullOrWhiteSpace(resourceAppId) && !string.IsNullOrWhiteSpace(scopeName)) { - _logger.LogDebug("Adding API permission '{ScopeName}' (resource: {ResourceAppId}) on '{AppName}' ({ObjectId})", scopeName, resourceAppId, apps.RemoteProxyAppName, apps.RemoteProxyObjectId); - var success = await _retryHelper.ExecuteWithRetryAsync( - async ct => await _graphApiService.AddRequiredResourceAccessAsync( - tenantId, apps.RemoteProxyObjectId, resourceAppId, remoteScopeId.Value, ct), - result => !result); - if (!success) + _logger.LogDebug("Looking up scope '{ScopeName}' on resource app {ResourceAppId}...", scopeName, resourceAppId); + var remoteScopeId = await _graphApiService!.GetOAuth2PermissionScopeIdAsync(tenantId, resourceAppId, scopeName); + if (remoteScopeId.HasValue) + { + _logger.LogDebug("Adding API permission '{ScopeName}' (resource: {ResourceAppId}) on '{AppName}' ({ObjectId})", scopeName, resourceAppId, apps.RemoteProxyAppName, apps.RemoteProxyObjectId); + var success = await _retryHelper.ExecuteWithRetryAsync( + async ct => await _graphApiService.AddRequiredResourceAccessAsync( + tenantId, apps.RemoteProxyObjectId, resourceAppId, remoteScopeId.Value, ct), + result => !result); + if (!success) + { + var msg = $"Failed to add API permission on RemoteProxy app '{apps.RemoteProxyAppName}' after retries."; + _logger.LogError(msg); + concurrentWarnings.Add(msg); + } + } + else { - var msg = $"Failed to add API permission on RemoteProxy app '{apps.RemoteProxyAppName}' after retries."; + var msg = $"Could not find scope '{scopeName}' on resource app {resourceAppId}. API permission not added to RemoteProxy app."; _logger.LogError(msg); - warnings.Add(msg); + concurrentWarnings.Add(msg); } } else { - var msg = $"Could not find scope '{scopeName}' on resource app {resourceAppId}. API permission not added to RemoteProxy app."; - _logger.LogError(msg); - warnings.Add(msg); + var msg = $"Could not parse resource app ID and scope from '{input.RemoteScopes}'. Expected format: api://{{appId}}/{{scopeName}}"; + _logger.LogWarning(msg); + concurrentWarnings.Add(msg); } } - else + catch (Exception ex) { - var msg = $"Could not parse resource app ID and scope from '{input.RemoteScopes}'. Expected format: api://{{appId}}/{{scopeName}}"; - _logger.LogWarning(msg); - warnings.Add(msg); + var msg = $"Failed to add API permissions on RemoteProxy app: {ex.Message}"; + _logger.LogError(msg); + concurrentWarnings.Add(msg); } - } - catch (Exception ex) - { - var msg = $"Failed to add API permissions on RemoteProxy app: {ex.Message}"; - _logger.LogError(msg); - warnings.Add(msg); - } + })); } var ppmiAppClientId = response.Server?.PpmiAppClientId; + Guid? ppmiScopeId = null; if (!string.IsNullOrWhiteSpace(ppmiAppClientId)) { _logger.LogDebug("PPMI app provisioned: {PpmiAppClientId}", ppmiAppClientId); - - var ppmiScopeId = await _graphApiService!.GetOAuth2PermissionScopeIdAsync( + ppmiScopeId = await _graphApiService!.GetOAuth2PermissionScopeIdAsync( tenantId, ppmiAppClientId, "Tools.ListInvoke.All"); - if (ppmiScopeId.HasValue) + } + + if (ppmiScopeId.HasValue) + { + tasks.Add(Task.Run(async () => { try { - _logger.LogDebug("Adding PPMI 'Tools.ListInvoke.All' permission (resource: {PpmiAppId}) on '{AppName}' ({ObjectId})", ppmiAppClientId, apps.A365AppName, apps.A365AppObjectId); - var a365Success = await _retryHelper.ExecuteWithRetryAsync( - async ct => await _graphApiService.AddRequiredResourceAccessAsync( - tenantId, apps.A365AppObjectId, ppmiAppClientId, ppmiScopeId.Value, ct), + _logger.LogDebug("Adding PPMI 'Tools.ListInvoke.All' permission on '{AppName}' ({ObjectId})", apps.A365AppName, apps.A365AppObjectId); + var success = await _retryHelper.ExecuteWithRetryAsync( + async ct => await _graphApiService!.AddRequiredResourceAccessAsync( + tenantId, apps.A365AppObjectId, ppmiAppClientId!, ppmiScopeId.Value, ct), result => !result); - if (!a365Success) + if (!success) { var msg = $"Failed to add PPMI permission on A365 Proxy app '{apps.A365AppName}' after retries."; _logger.LogError(msg); - warnings.Add(msg); + concurrentWarnings.Add(msg); } else { @@ -917,40 +927,48 @@ private async Task ConfigureApiPermissionsAsync( { var msg = $"Failed to add PPMI permission on A365 Proxy app: {ex.Message}"; _logger.LogError(msg); - warnings.Add(msg); + concurrentWarnings.Add(msg); } + })); - if (apps.PublicClientsObjectId != null) + if (apps.PublicClientsObjectId != null) + { + tasks.Add(Task.Run(async () => { try { - _logger.LogDebug("Adding PPMI 'Tools.ListInvoke.All' permission (resource: {PpmiAppId}) on '{AppName}' ({ObjectId})", ppmiAppClientId, apps.PublicClientsAppName, apps.PublicClientsObjectId); - var copilotSuccess = await _retryHelper.ExecuteWithRetryAsync( - async ct => await _graphApiService.AddRequiredResourceAccessAsync( - tenantId, apps.PublicClientsObjectId, ppmiAppClientId, ppmiScopeId.Value, ct), + _logger.LogDebug("Adding PPMI 'Tools.ListInvoke.All' permission on '{AppName}' ({ObjectId})", apps.PublicClientsAppName, apps.PublicClientsObjectId); + var success = await _retryHelper.ExecuteWithRetryAsync( + async ct => await _graphApiService!.AddRequiredResourceAccessAsync( + tenantId, apps.PublicClientsObjectId, ppmiAppClientId!, ppmiScopeId.Value, ct), result => !result); - if (!copilotSuccess) + if (!success) { var msg = $"Failed to add PPMI permission on Public Clients app '{apps.PublicClientsAppName}' after retries."; _logger.LogError(msg); - warnings.Add(msg); + concurrentWarnings.Add(msg); } } catch (Exception ex) { var msg = $"Failed to add PPMI permission on Public Clients app: {ex.Message}"; _logger.LogError(msg); - warnings.Add(msg); + concurrentWarnings.Add(msg); } - } - } - else - { - var msg = $"Could not find 'Tools.ListInvoke.All' scope on PPMI app {ppmiAppClientId}. API permissions not added."; - _logger.LogError(msg); - warnings.Add(msg); + })); } } + else if (!string.IsNullOrWhiteSpace(ppmiAppClientId)) + { + var msg = $"Could not find 'Tools.ListInvoke.All' scope on PPMI app {ppmiAppClientId}. API permissions not added."; + _logger.LogError(msg); + concurrentWarnings.Add(msg); + } + + await Task.WhenAll(tasks); + + foreach (var w in concurrentWarnings) + warnings.Add(w); } From 3c1ce3320de78a80db8f5f41be35a9a4d341150f Mon Sep 17 00:00:00 2001 From: Bhaarath Raguru Date: Thu, 30 Apr 2026 11:52:17 -0700 Subject: [PATCH 10/14] fix: address PR review comments - improve log messages and error context - Log full redirect URI count and list instead of singular URI - Add scope name and resource app ID to RemoteProxy permission error message Co-Authored-By: Claude Opus 4.6 --- .../Commands/RegisterCommandExecutor.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs index 3f3b28fa..5c16b557 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs @@ -648,7 +648,12 @@ private void DisplayRegistrationSummary(ResolvedInput input) } else { - _logger.LogDebug("Set redirect URI on '{AppName}' ({ObjectId}): {Uri}", publicClientsAppName, publicClientsObjectId, copilotRedirectUri); + _logger.LogDebug( + "Set {RedirectUriCount} redirect URIs on '{AppName}' ({ObjectId}): {RedirectUris}", + publicClientUris.Length, + publicClientsAppName, + publicClientsObjectId, + string.Join(", ", publicClientUris)); } } catch (Exception ex) @@ -864,7 +869,7 @@ private async Task ConfigureEntraAppsAsync( result => !result); if (!success) { - var msg = $"Failed to add API permission on RemoteProxy app '{apps.RemoteProxyAppName}' after retries."; + var msg = $"Failed to add API permission '{scopeName}' from resource app {resourceAppId} on RemoteProxy app '{apps.RemoteProxyAppName}' after retries."; _logger.LogError(msg); concurrentWarnings.Add(msg); } From e7a6f8ea7a11738ec1b16d1e884d3da2a62af40f Mon Sep 17 00:00:00 2001 From: Bhaarath Raguru Date: Thu, 30 Apr 2026 12:07:06 -0700 Subject: [PATCH 11/14] fix: address PR review - remove Task.Run, split warnings, retry scope lookups - Remove unnecessary Task.Run wrappers around async I/O operations; extracted to dedicated async methods called directly for parallelism - Split RemoteProxy else-if warning into separate messages for missing redirect URI vs missing ObjectId - Wrap GetOAuth2PermissionScopeIdAsync calls in RetryHelper for transient failure resilience - Extract AddPpmiPermissionAsync for reuse across A365 and PublicClients apps Co-Authored-By: Claude Opus 4.6 --- .../Commands/RegisterCommandExecutor.cs | 331 +++++++++--------- 1 file changed, 168 insertions(+), 163 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs index 5c16b557..35739c57 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs @@ -759,35 +759,7 @@ private async Task ConfigureEntraAppsAsync( if (!string.IsNullOrWhiteSpace(a365RedirectUri)) { - tasks.Add(Task.Run(async () => - { - try - { - var a365TcUri = DevelopMcpCommand.AddTcPrefix(a365RedirectUri); - var a365NonTcUri = DevelopMcpCommand.RemoveTcPrefix(a365RedirectUri); - var a365Uris = DevelopMcpCommand.BuildRedirectUriList(a365RedirectUri, a365TcUri, a365NonTcUri); - _logger.LogDebug("Updating redirect URIs on '{AppName}' ({ObjectId})", apps.A365AppName, apps.A365AppObjectId); - var success = await _retryHelper.ExecuteWithRetryAsync( - async ct => await _graphApiService!.UpdateAppRedirectUrisAsync(tenantId, apps.A365AppObjectId, a365Uris, ct), - result => !result); - if (!success) - { - var msg = $"Failed to update redirect URIs on A365 Proxy app '{apps.A365AppName}' after retries."; - _logger.LogError(msg); - concurrentWarnings.Add(msg); - } - else - { - _logger.LogInformation("Updated redirect URIs on '{AppName}'", apps.A365AppName); - } - } - catch (Exception ex) - { - var msg = $"Failed to update redirect URIs on A365 Proxy app: {ex.Message}"; - _logger.LogError(msg); - concurrentWarnings.Add(msg); - } - })); + tasks.Add(UpdateA365RedirectUrisAsync(tenantId, apps, a365RedirectUri, concurrentWarnings)); } else { @@ -798,103 +770,24 @@ private async Task ConfigureEntraAppsAsync( if (input.IsEntra && !string.IsNullOrWhiteSpace(remoteRedirectUri) && apps.RemoteProxyObjectId != null) { - tasks.Add(Task.Run(async () => - { - try - { - var remoteTcUri = DevelopMcpCommand.AddTcPrefix(remoteRedirectUri); - var remoteNonTcUri = DevelopMcpCommand.RemoveTcPrefix(remoteRedirectUri); - var remoteUris = DevelopMcpCommand.BuildRedirectUriList(remoteRedirectUri, remoteTcUri, remoteNonTcUri); - _logger.LogDebug("Updating redirect URIs on '{AppName}' ({ObjectId})", apps.RemoteProxyAppName, apps.RemoteProxyObjectId); - var success = await _retryHelper.ExecuteWithRetryAsync( - async ct => await _graphApiService!.UpdateAppRedirectUrisAsync(tenantId, apps.RemoteProxyObjectId, remoteUris, ct), - result => !result); - if (!success) - { - var msg = $"Failed to update redirect URIs on Remote Proxy app '{apps.RemoteProxyAppName}' after retries."; - _logger.LogError(msg); - concurrentWarnings.Add(msg); - } - else - { - _logger.LogInformation("Updated redirect URIs on '{AppName}'", apps.RemoteProxyAppName); - } - } - catch (Exception ex) - { - var msg = $"Failed to update redirect URIs on Remote Proxy app: {ex.Message}"; - _logger.LogError(msg); - concurrentWarnings.Add(msg); - } - })); + tasks.Add(UpdateRemoteProxyRedirectUrisAsync(tenantId, apps, remoteRedirectUri, concurrentWarnings)); } - else if (input.IsEntra) + else if (input.IsEntra && string.IsNullOrWhiteSpace(remoteRedirectUri)) { var msg = "Remote MCP Proxy redirect URI was not returned by the server. Redirect URI configuration skipped."; _logger.LogWarning(msg); concurrentWarnings.Add(msg); } + else if (input.IsEntra && apps.RemoteProxyObjectId == null) + { + var msg = "Remote Proxy Entra app was not created. Redirect URI configuration skipped."; + _logger.LogWarning(msg); + concurrentWarnings.Add(msg); + } if (input.IsEntra && apps.RemoteProxyObjectId != null && !string.IsNullOrWhiteSpace(input.RemoteScopes)) { - tasks.Add(Task.Run(async () => - { - try - { - var scopeUri = input.RemoteScopes.Trim(); - string? resourceAppId = null; - string? scopeName = null; - - if (scopeUri.StartsWith("api://", StringComparison.OrdinalIgnoreCase)) - { - var path = scopeUri.Substring("api://".Length); - var slashIndex = path.IndexOf('/'); - if (slashIndex > 0) - { - resourceAppId = path.Substring(0, slashIndex); - scopeName = path.Substring(slashIndex + 1); - } - } - - if (!string.IsNullOrWhiteSpace(resourceAppId) && !string.IsNullOrWhiteSpace(scopeName)) - { - _logger.LogDebug("Looking up scope '{ScopeName}' on resource app {ResourceAppId}...", scopeName, resourceAppId); - var remoteScopeId = await _graphApiService!.GetOAuth2PermissionScopeIdAsync(tenantId, resourceAppId, scopeName); - if (remoteScopeId.HasValue) - { - _logger.LogDebug("Adding API permission '{ScopeName}' (resource: {ResourceAppId}) on '{AppName}' ({ObjectId})", scopeName, resourceAppId, apps.RemoteProxyAppName, apps.RemoteProxyObjectId); - var success = await _retryHelper.ExecuteWithRetryAsync( - async ct => await _graphApiService.AddRequiredResourceAccessAsync( - tenantId, apps.RemoteProxyObjectId, resourceAppId, remoteScopeId.Value, ct), - result => !result); - if (!success) - { - var msg = $"Failed to add API permission '{scopeName}' from resource app {resourceAppId} on RemoteProxy app '{apps.RemoteProxyAppName}' after retries."; - _logger.LogError(msg); - concurrentWarnings.Add(msg); - } - } - else - { - var msg = $"Could not find scope '{scopeName}' on resource app {resourceAppId}. API permission not added to RemoteProxy app."; - _logger.LogError(msg); - concurrentWarnings.Add(msg); - } - } - else - { - var msg = $"Could not parse resource app ID and scope from '{input.RemoteScopes}'. Expected format: api://{{appId}}/{{scopeName}}"; - _logger.LogWarning(msg); - concurrentWarnings.Add(msg); - } - } - catch (Exception ex) - { - var msg = $"Failed to add API permissions on RemoteProxy app: {ex.Message}"; - _logger.LogError(msg); - concurrentWarnings.Add(msg); - } - })); + tasks.Add(AddRemoteProxyScopePermissionAsync(tenantId, input, apps, concurrentWarnings)); } var ppmiAppClientId = response.Server?.PpmiAppClientId; @@ -902,80 +795,192 @@ private async Task ConfigureEntraAppsAsync( if (!string.IsNullOrWhiteSpace(ppmiAppClientId)) { _logger.LogDebug("PPMI app provisioned: {PpmiAppClientId}", ppmiAppClientId); - ppmiScopeId = await _graphApiService!.GetOAuth2PermissionScopeIdAsync( - tenantId, ppmiAppClientId, "Tools.ListInvoke.All"); + ppmiScopeId = await _retryHelper.ExecuteWithRetryAsync( + async ct => await _graphApiService!.GetOAuth2PermissionScopeIdAsync( + tenantId, ppmiAppClientId, "Tools.ListInvoke.All", ct), + result => !result.HasValue); } if (ppmiScopeId.HasValue) { - tasks.Add(Task.Run(async () => + tasks.Add(AddPpmiPermissionAsync(tenantId, apps.A365AppObjectId, apps.A365AppName, ppmiAppClientId!, ppmiScopeId.Value, concurrentWarnings)); + + if (apps.PublicClientsObjectId != null) + { + tasks.Add(AddPpmiPermissionAsync(tenantId, apps.PublicClientsObjectId, apps.PublicClientsAppName, ppmiAppClientId!, ppmiScopeId.Value, concurrentWarnings)); + } + } + else if (!string.IsNullOrWhiteSpace(ppmiAppClientId)) + { + var msg = $"Could not find 'Tools.ListInvoke.All' scope on PPMI app {ppmiAppClientId}. API permissions not added."; + _logger.LogError(msg); + concurrentWarnings.Add(msg); + } + + await Task.WhenAll(tasks); + + foreach (var w in concurrentWarnings) + warnings.Add(w); + } + + + private async Task UpdateA365RedirectUrisAsync( + string tenantId, EntraAppSet apps, string a365RedirectUri, + System.Collections.Concurrent.ConcurrentBag concurrentWarnings) + { + try + { + var a365TcUri = DevelopMcpCommand.AddTcPrefix(a365RedirectUri); + var a365NonTcUri = DevelopMcpCommand.RemoveTcPrefix(a365RedirectUri); + var a365Uris = DevelopMcpCommand.BuildRedirectUriList(a365RedirectUri, a365TcUri, a365NonTcUri); + _logger.LogDebug("Updating redirect URIs on '{AppName}' ({ObjectId})", apps.A365AppName, apps.A365AppObjectId); + var success = await _retryHelper.ExecuteWithRetryAsync( + async ct => await _graphApiService!.UpdateAppRedirectUrisAsync(tenantId, apps.A365AppObjectId, a365Uris, ct), + result => !result); + if (!success) + { + var msg = $"Failed to update redirect URIs on A365 Proxy app '{apps.A365AppName}' after retries."; + _logger.LogError(msg); + concurrentWarnings.Add(msg); + } + else + { + _logger.LogInformation("Updated redirect URIs on '{AppName}'", apps.A365AppName); + } + } + catch (Exception ex) + { + var msg = $"Failed to update redirect URIs on A365 Proxy app: {ex.Message}"; + _logger.LogError(msg); + concurrentWarnings.Add(msg); + } + } + + private async Task UpdateRemoteProxyRedirectUrisAsync( + string tenantId, EntraAppSet apps, string remoteRedirectUri, + System.Collections.Concurrent.ConcurrentBag concurrentWarnings) + { + try + { + var remoteTcUri = DevelopMcpCommand.AddTcPrefix(remoteRedirectUri); + var remoteNonTcUri = DevelopMcpCommand.RemoveTcPrefix(remoteRedirectUri); + var remoteUris = DevelopMcpCommand.BuildRedirectUriList(remoteRedirectUri, remoteTcUri, remoteNonTcUri); + _logger.LogDebug("Updating redirect URIs on '{AppName}' ({ObjectId})", apps.RemoteProxyAppName, apps.RemoteProxyObjectId); + var success = await _retryHelper.ExecuteWithRetryAsync( + async ct => await _graphApiService!.UpdateAppRedirectUrisAsync(tenantId, apps.RemoteProxyObjectId!, remoteUris, ct), + result => !result); + if (!success) + { + var msg = $"Failed to update redirect URIs on Remote Proxy app '{apps.RemoteProxyAppName}' after retries."; + _logger.LogError(msg); + concurrentWarnings.Add(msg); + } + else + { + _logger.LogInformation("Updated redirect URIs on '{AppName}'", apps.RemoteProxyAppName); + } + } + catch (Exception ex) + { + var msg = $"Failed to update redirect URIs on Remote Proxy app: {ex.Message}"; + _logger.LogError(msg); + concurrentWarnings.Add(msg); + } + } + + private async Task AddRemoteProxyScopePermissionAsync( + string tenantId, ResolvedInput input, EntraAppSet apps, + System.Collections.Concurrent.ConcurrentBag concurrentWarnings) + { + try + { + var scopeUri = input.RemoteScopes!.Trim(); + string? resourceAppId = null; + string? scopeName = null; + + if (scopeUri.StartsWith("api://", StringComparison.OrdinalIgnoreCase)) + { + var path = scopeUri.Substring("api://".Length); + var slashIndex = path.IndexOf('/'); + if (slashIndex > 0) + { + resourceAppId = path.Substring(0, slashIndex); + scopeName = path.Substring(slashIndex + 1); + } + } + + if (!string.IsNullOrWhiteSpace(resourceAppId) && !string.IsNullOrWhiteSpace(scopeName)) { - try + _logger.LogDebug("Looking up scope '{ScopeName}' on resource app {ResourceAppId}...", scopeName, resourceAppId); + var remoteScopeId = await _retryHelper.ExecuteWithRetryAsync( + async ct => await _graphApiService!.GetOAuth2PermissionScopeIdAsync(tenantId, resourceAppId, scopeName, ct), + result => !result.HasValue); + if (remoteScopeId.HasValue) { - _logger.LogDebug("Adding PPMI 'Tools.ListInvoke.All' permission on '{AppName}' ({ObjectId})", apps.A365AppName, apps.A365AppObjectId); + _logger.LogDebug("Adding API permission '{ScopeName}' (resource: {ResourceAppId}) on '{AppName}' ({ObjectId})", scopeName, resourceAppId, apps.RemoteProxyAppName, apps.RemoteProxyObjectId); var success = await _retryHelper.ExecuteWithRetryAsync( async ct => await _graphApiService!.AddRequiredResourceAccessAsync( - tenantId, apps.A365AppObjectId, ppmiAppClientId!, ppmiScopeId.Value, ct), + tenantId, apps.RemoteProxyObjectId!, resourceAppId, remoteScopeId.Value, ct), result => !result); if (!success) { - var msg = $"Failed to add PPMI permission on A365 Proxy app '{apps.A365AppName}' after retries."; + var msg = $"Failed to add API permission '{scopeName}' from resource app {resourceAppId} on RemoteProxy app '{apps.RemoteProxyAppName}' after retries."; _logger.LogError(msg); concurrentWarnings.Add(msg); } - else - { - _logger.LogInformation("Added API permission on '{AppName}'", apps.A365AppName); - } } - catch (Exception ex) + else { - var msg = $"Failed to add PPMI permission on A365 Proxy app: {ex.Message}"; + var msg = $"Could not find scope '{scopeName}' on resource app {resourceAppId}. API permission not added to RemoteProxy app."; _logger.LogError(msg); concurrentWarnings.Add(msg); } - })); - - if (apps.PublicClientsObjectId != null) + } + else { - tasks.Add(Task.Run(async () => - { - try - { - _logger.LogDebug("Adding PPMI 'Tools.ListInvoke.All' permission on '{AppName}' ({ObjectId})", apps.PublicClientsAppName, apps.PublicClientsObjectId); - var success = await _retryHelper.ExecuteWithRetryAsync( - async ct => await _graphApiService!.AddRequiredResourceAccessAsync( - tenantId, apps.PublicClientsObjectId, ppmiAppClientId!, ppmiScopeId.Value, ct), - result => !result); - if (!success) - { - var msg = $"Failed to add PPMI permission on Public Clients app '{apps.PublicClientsAppName}' after retries."; - _logger.LogError(msg); - concurrentWarnings.Add(msg); - } - } - catch (Exception ex) - { - var msg = $"Failed to add PPMI permission on Public Clients app: {ex.Message}"; - _logger.LogError(msg); - concurrentWarnings.Add(msg); - } - })); + var msg = $"Could not parse resource app ID and scope from '{input.RemoteScopes}'. Expected format: api://{{appId}}/{{scopeName}}"; + _logger.LogWarning(msg); + concurrentWarnings.Add(msg); } } - else if (!string.IsNullOrWhiteSpace(ppmiAppClientId)) + catch (Exception ex) { - var msg = $"Could not find 'Tools.ListInvoke.All' scope on PPMI app {ppmiAppClientId}. API permissions not added."; + var msg = $"Failed to add API permissions on RemoteProxy app: {ex.Message}"; _logger.LogError(msg); concurrentWarnings.Add(msg); } - - await Task.WhenAll(tasks); - - foreach (var w in concurrentWarnings) - warnings.Add(w); } + private async Task AddPpmiPermissionAsync( + string tenantId, string appObjectId, string appName, + string ppmiAppClientId, Guid ppmiScopeId, + System.Collections.Concurrent.ConcurrentBag concurrentWarnings) + { + try + { + _logger.LogDebug("Adding PPMI 'Tools.ListInvoke.All' permission on '{AppName}' ({ObjectId})", appName, appObjectId); + var success = await _retryHelper.ExecuteWithRetryAsync( + async ct => await _graphApiService!.AddRequiredResourceAccessAsync( + tenantId, appObjectId, ppmiAppClientId, ppmiScopeId, ct), + result => !result); + if (!success) + { + var msg = $"Failed to add PPMI permission on '{appName}' after retries."; + _logger.LogError(msg); + concurrentWarnings.Add(msg); + } + else + { + _logger.LogInformation("Added API permission on '{AppName}'", appName); + } + } + catch (Exception ex) + { + var msg = $"Failed to add PPMI permission on '{appName}': {ex.Message}"; + _logger.LogError(msg); + concurrentWarnings.Add(msg); + } + } private void DisplayResults( ResolvedInput input, string? remoteRedirectUri, List warnings) From 0b6620e5228df6e9cd28e1486666f42e669d58d7 Mon Sep 17 00:00:00 2001 From: Bhaarath Raguru Date: Thu, 30 Apr 2026 14:27:59 -0700 Subject: [PATCH 12/14] fix: prevent RetryExhaustedException crash on nullable scope lookups RetryHelper throws RetryExhaustedException when shouldRetry keeps returning true and the final result is null. For Guid? scope lookups this caused an unhandled crash instead of a graceful warning. Replaced shouldRetry-based retry with exception-only overload wrapped in try/catch for both PPMI and remote scope lookups. Co-Authored-By: Claude Opus 4.6 --- .../Commands/RegisterCommandExecutor.cs | 34 ++++++++++++++----- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs index 35739c57..fd0aec00 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs @@ -795,10 +795,18 @@ private async Task ConfigureEntraAppsAsync( if (!string.IsNullOrWhiteSpace(ppmiAppClientId)) { _logger.LogDebug("PPMI app provisioned: {PpmiAppClientId}", ppmiAppClientId); - ppmiScopeId = await _retryHelper.ExecuteWithRetryAsync( - async ct => await _graphApiService!.GetOAuth2PermissionScopeIdAsync( - tenantId, ppmiAppClientId, "Tools.ListInvoke.All", ct), - result => !result.HasValue); + try + { + ppmiScopeId = await _retryHelper.ExecuteWithRetryAsync( + async ct => await _graphApiService!.GetOAuth2PermissionScopeIdAsync( + tenantId, ppmiAppClientId, "Tools.ListInvoke.All", ct)); + } + catch (Exception ex) + { + var msg = $"Could not find 'Tools.ListInvoke.All' scope on PPMI app {ppmiAppClientId} after retries: {ex.Message}. API permissions not added."; + _logger.LogError(msg); + concurrentWarnings.Add(msg); + } } if (ppmiScopeId.HasValue) @@ -810,7 +818,7 @@ private async Task ConfigureEntraAppsAsync( tasks.Add(AddPpmiPermissionAsync(tenantId, apps.PublicClientsObjectId, apps.PublicClientsAppName, ppmiAppClientId!, ppmiScopeId.Value, concurrentWarnings)); } } - else if (!string.IsNullOrWhiteSpace(ppmiAppClientId)) + else if (!string.IsNullOrWhiteSpace(ppmiAppClientId) && ppmiScopeId == null) { var msg = $"Could not find 'Tools.ListInvoke.All' scope on PPMI app {ppmiAppClientId}. API permissions not added."; _logger.LogError(msg); @@ -912,9 +920,19 @@ private async Task AddRemoteProxyScopePermissionAsync( if (!string.IsNullOrWhiteSpace(resourceAppId) && !string.IsNullOrWhiteSpace(scopeName)) { _logger.LogDebug("Looking up scope '{ScopeName}' on resource app {ResourceAppId}...", scopeName, resourceAppId); - var remoteScopeId = await _retryHelper.ExecuteWithRetryAsync( - async ct => await _graphApiService!.GetOAuth2PermissionScopeIdAsync(tenantId, resourceAppId, scopeName, ct), - result => !result.HasValue); + Guid? remoteScopeId = null; + try + { + remoteScopeId = await _retryHelper.ExecuteWithRetryAsync( + async ct => await _graphApiService!.GetOAuth2PermissionScopeIdAsync(tenantId, resourceAppId, scopeName, ct)); + } + catch (Exception ex) + { + var msg = $"Failed to look up scope '{scopeName}' on resource app {resourceAppId} after retries: {ex.Message}. API permission not added to RemoteProxy app."; + _logger.LogError(msg); + concurrentWarnings.Add(msg); + } + if (remoteScopeId.HasValue) { _logger.LogDebug("Adding API permission '{ScopeName}' (resource: {ResourceAppId}) on '{AppName}' ({ObjectId})", scopeName, resourceAppId, apps.RemoteProxyAppName, apps.RemoteProxyObjectId); From c89fabfd909c10816056849ae8b0664c54a732f7 Mon Sep 17 00:00:00 2001 From: Bhaarath Raguru Date: Thu, 30 Apr 2026 14:35:13 -0700 Subject: [PATCH 13/14] fix: thread CancellationToken from command handler through retries Pass context.GetCancellationToken() from DevelopMcpCommand SetHandler through ExecuteAsync, ConfigureEntraAppsAsync, and all helper methods to RetryHelper. Ctrl+C now cancels in-flight retries instead of burning through all 5 attempts (up to 93s). Co-Authored-By: Claude Opus 4.6 --- .../Commands/DevelopMcpCommand.cs | 2 +- .../Commands/RegisterCommandExecutor.cs | 64 +++++++++++-------- 2 files changed, 40 insertions(+), 26 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs index 3695ff7e..fd66a4fc 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs @@ -885,7 +885,7 @@ private static Command CreateRegisterExternalMcpServerSubcommand( DryRun: context.ParseResult.GetValueForOption(dryRunOption)); var executor = new RegisterCommandExecutor(logger, toolingService, graphApiService); - await executor.ExecuteAsync(args); + await executor.ExecuteAsync(args, context.GetCancellationToken()); }); return command; diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs index fd0aec00..f31c88fc 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs @@ -93,7 +93,7 @@ private sealed record EntraAppSet( string? PublicClientsObjectId, string PublicClientsAppName); - internal async Task ExecuteAsync(RawRegisterArgs args) + internal async Task ExecuteAsync(RawRegisterArgs args, CancellationToken ct = default) { var input = await ResolveInputsAsync(args); if (input is null) return; @@ -119,6 +119,8 @@ internal async Task ExecuteAsync(RawRegisterArgs args) Console.WriteLine(); + ct.ThrowIfCancellationRequested(); + await _toolingService.LogRegisterUsageAsync(input.ServerName, input.AuthType, input.ToolList.Count); Console.WriteLine($"Registering MCP server '{input.ServerName}'..."); @@ -135,6 +137,8 @@ internal async Task ExecuteAsync(RawRegisterArgs args) var apps = await CreateEntraAppsAsync(input, tenantId, warnings); if (apps is null) return; + ct.ThrowIfCancellationRequested(); + var addRequest = BuildRequest(input, apps); AddMcpServerResponse? addResponse; @@ -177,7 +181,7 @@ internal async Task ExecuteAsync(RawRegisterArgs args) _logger.LogDebug("Successfully added MCP server {ServerName}", input.ServerName); - await ConfigureEntraAppsAsync(input, apps, addResponse, tenantId, warnings); + await ConfigureEntraAppsAsync(input, apps, addResponse, tenantId, warnings, ct); DisplayResults(input, addResponse.Server?.RemoteMCPServerProxyRedirectUri, warnings); } @@ -749,7 +753,7 @@ private static AddMcpServerRequest BuildRequest(ResolvedInput input, EntraAppSet private async Task ConfigureEntraAppsAsync( ResolvedInput input, EntraAppSet apps, AddMcpServerResponse response, - string tenantId, List warnings) + string tenantId, List warnings, CancellationToken ct = default) { var tasks = new List(); var concurrentWarnings = new System.Collections.Concurrent.ConcurrentBag(); @@ -759,7 +763,7 @@ private async Task ConfigureEntraAppsAsync( if (!string.IsNullOrWhiteSpace(a365RedirectUri)) { - tasks.Add(UpdateA365RedirectUrisAsync(tenantId, apps, a365RedirectUri, concurrentWarnings)); + tasks.Add(UpdateA365RedirectUrisAsync(tenantId, apps, a365RedirectUri, concurrentWarnings, ct)); } else { @@ -770,7 +774,7 @@ private async Task ConfigureEntraAppsAsync( if (input.IsEntra && !string.IsNullOrWhiteSpace(remoteRedirectUri) && apps.RemoteProxyObjectId != null) { - tasks.Add(UpdateRemoteProxyRedirectUrisAsync(tenantId, apps, remoteRedirectUri, concurrentWarnings)); + tasks.Add(UpdateRemoteProxyRedirectUrisAsync(tenantId, apps, remoteRedirectUri, concurrentWarnings, ct)); } else if (input.IsEntra && string.IsNullOrWhiteSpace(remoteRedirectUri)) { @@ -787,7 +791,7 @@ private async Task ConfigureEntraAppsAsync( if (input.IsEntra && apps.RemoteProxyObjectId != null && !string.IsNullOrWhiteSpace(input.RemoteScopes)) { - tasks.Add(AddRemoteProxyScopePermissionAsync(tenantId, input, apps, concurrentWarnings)); + tasks.Add(AddRemoteProxyScopePermissionAsync(tenantId, input, apps, concurrentWarnings, ct)); } var ppmiAppClientId = response.Server?.PpmiAppClientId; @@ -798,8 +802,9 @@ private async Task ConfigureEntraAppsAsync( try { ppmiScopeId = await _retryHelper.ExecuteWithRetryAsync( - async ct => await _graphApiService!.GetOAuth2PermissionScopeIdAsync( - tenantId, ppmiAppClientId, "Tools.ListInvoke.All", ct)); + async retryCt => await _graphApiService!.GetOAuth2PermissionScopeIdAsync( + tenantId, ppmiAppClientId, "Tools.ListInvoke.All", retryCt), + cancellationToken: ct); } catch (Exception ex) { @@ -811,11 +816,11 @@ private async Task ConfigureEntraAppsAsync( if (ppmiScopeId.HasValue) { - tasks.Add(AddPpmiPermissionAsync(tenantId, apps.A365AppObjectId, apps.A365AppName, ppmiAppClientId!, ppmiScopeId.Value, concurrentWarnings)); + tasks.Add(AddPpmiPermissionAsync(tenantId, apps.A365AppObjectId, apps.A365AppName, ppmiAppClientId!, ppmiScopeId.Value, concurrentWarnings, ct)); if (apps.PublicClientsObjectId != null) { - tasks.Add(AddPpmiPermissionAsync(tenantId, apps.PublicClientsObjectId, apps.PublicClientsAppName, ppmiAppClientId!, ppmiScopeId.Value, concurrentWarnings)); + tasks.Add(AddPpmiPermissionAsync(tenantId, apps.PublicClientsObjectId, apps.PublicClientsAppName, ppmiAppClientId!, ppmiScopeId.Value, concurrentWarnings, ct)); } } else if (!string.IsNullOrWhiteSpace(ppmiAppClientId) && ppmiScopeId == null) @@ -834,7 +839,8 @@ private async Task ConfigureEntraAppsAsync( private async Task UpdateA365RedirectUrisAsync( string tenantId, EntraAppSet apps, string a365RedirectUri, - System.Collections.Concurrent.ConcurrentBag concurrentWarnings) + System.Collections.Concurrent.ConcurrentBag concurrentWarnings, + CancellationToken ct = default) { try { @@ -843,8 +849,9 @@ private async Task UpdateA365RedirectUrisAsync( var a365Uris = DevelopMcpCommand.BuildRedirectUriList(a365RedirectUri, a365TcUri, a365NonTcUri); _logger.LogDebug("Updating redirect URIs on '{AppName}' ({ObjectId})", apps.A365AppName, apps.A365AppObjectId); var success = await _retryHelper.ExecuteWithRetryAsync( - async ct => await _graphApiService!.UpdateAppRedirectUrisAsync(tenantId, apps.A365AppObjectId, a365Uris, ct), - result => !result); + async retryCt => await _graphApiService!.UpdateAppRedirectUrisAsync(tenantId, apps.A365AppObjectId, a365Uris, retryCt), + result => !result, + cancellationToken: ct); if (!success) { var msg = $"Failed to update redirect URIs on A365 Proxy app '{apps.A365AppName}' after retries."; @@ -866,7 +873,8 @@ private async Task UpdateA365RedirectUrisAsync( private async Task UpdateRemoteProxyRedirectUrisAsync( string tenantId, EntraAppSet apps, string remoteRedirectUri, - System.Collections.Concurrent.ConcurrentBag concurrentWarnings) + System.Collections.Concurrent.ConcurrentBag concurrentWarnings, + CancellationToken ct = default) { try { @@ -875,8 +883,9 @@ private async Task UpdateRemoteProxyRedirectUrisAsync( var remoteUris = DevelopMcpCommand.BuildRedirectUriList(remoteRedirectUri, remoteTcUri, remoteNonTcUri); _logger.LogDebug("Updating redirect URIs on '{AppName}' ({ObjectId})", apps.RemoteProxyAppName, apps.RemoteProxyObjectId); var success = await _retryHelper.ExecuteWithRetryAsync( - async ct => await _graphApiService!.UpdateAppRedirectUrisAsync(tenantId, apps.RemoteProxyObjectId!, remoteUris, ct), - result => !result); + async retryCt => await _graphApiService!.UpdateAppRedirectUrisAsync(tenantId, apps.RemoteProxyObjectId!, remoteUris, retryCt), + result => !result, + cancellationToken: ct); if (!success) { var msg = $"Failed to update redirect URIs on Remote Proxy app '{apps.RemoteProxyAppName}' after retries."; @@ -898,7 +907,8 @@ private async Task UpdateRemoteProxyRedirectUrisAsync( private async Task AddRemoteProxyScopePermissionAsync( string tenantId, ResolvedInput input, EntraAppSet apps, - System.Collections.Concurrent.ConcurrentBag concurrentWarnings) + System.Collections.Concurrent.ConcurrentBag concurrentWarnings, + CancellationToken ct = default) { try { @@ -924,7 +934,8 @@ private async Task AddRemoteProxyScopePermissionAsync( try { remoteScopeId = await _retryHelper.ExecuteWithRetryAsync( - async ct => await _graphApiService!.GetOAuth2PermissionScopeIdAsync(tenantId, resourceAppId, scopeName, ct)); + async retryCt => await _graphApiService!.GetOAuth2PermissionScopeIdAsync(tenantId, resourceAppId, scopeName, retryCt), + cancellationToken: ct); } catch (Exception ex) { @@ -937,9 +948,10 @@ private async Task AddRemoteProxyScopePermissionAsync( { _logger.LogDebug("Adding API permission '{ScopeName}' (resource: {ResourceAppId}) on '{AppName}' ({ObjectId})", scopeName, resourceAppId, apps.RemoteProxyAppName, apps.RemoteProxyObjectId); var success = await _retryHelper.ExecuteWithRetryAsync( - async ct => await _graphApiService!.AddRequiredResourceAccessAsync( - tenantId, apps.RemoteProxyObjectId!, resourceAppId, remoteScopeId.Value, ct), - result => !result); + async retryCt => await _graphApiService!.AddRequiredResourceAccessAsync( + tenantId, apps.RemoteProxyObjectId!, resourceAppId, remoteScopeId.Value, retryCt), + result => !result, + cancellationToken: ct); if (!success) { var msg = $"Failed to add API permission '{scopeName}' from resource app {resourceAppId} on RemoteProxy app '{apps.RemoteProxyAppName}' after retries."; @@ -972,15 +984,17 @@ private async Task AddRemoteProxyScopePermissionAsync( private async Task AddPpmiPermissionAsync( string tenantId, string appObjectId, string appName, string ppmiAppClientId, Guid ppmiScopeId, - System.Collections.Concurrent.ConcurrentBag concurrentWarnings) + System.Collections.Concurrent.ConcurrentBag concurrentWarnings, + CancellationToken ct = default) { try { _logger.LogDebug("Adding PPMI 'Tools.ListInvoke.All' permission on '{AppName}' ({ObjectId})", appName, appObjectId); var success = await _retryHelper.ExecuteWithRetryAsync( - async ct => await _graphApiService!.AddRequiredResourceAccessAsync( - tenantId, appObjectId, ppmiAppClientId, ppmiScopeId, ct), - result => !result); + async retryCt => await _graphApiService!.AddRequiredResourceAccessAsync( + tenantId, appObjectId, ppmiAppClientId, ppmiScopeId, retryCt), + result => !result, + cancellationToken: ct); if (!success) { var msg = $"Failed to add PPMI permission on '{appName}' after retries."; From 502c535e5d63027990e690a1a7133c479b3ab96a Mon Sep 17 00:00:00 2001 From: Bhaarath Raguru Date: Thu, 30 Apr 2026 14:41:18 -0700 Subject: [PATCH 14/14] fix: retry scope lookups on null results for eventual consistency Scope lookups can return null due to Entra replication lag after app creation. Re-added shouldRetry predicate (result => !result.HasValue) while keeping try/catch for RetryExhaustedException when scope truly does not exist after all retries. Co-Authored-By: Claude Opus 4.6 --- .../Commands/RegisterCommandExecutor.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs index f31c88fc..5f2bfd13 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/RegisterCommandExecutor.cs @@ -804,6 +804,7 @@ private async Task ConfigureEntraAppsAsync( ppmiScopeId = await _retryHelper.ExecuteWithRetryAsync( async retryCt => await _graphApiService!.GetOAuth2PermissionScopeIdAsync( tenantId, ppmiAppClientId, "Tools.ListInvoke.All", retryCt), + result => !result.HasValue, cancellationToken: ct); } catch (Exception ex) @@ -935,6 +936,7 @@ private async Task AddRemoteProxyScopePermissionAsync( { remoteScopeId = await _retryHelper.ExecuteWithRetryAsync( async retryCt => await _graphApiService!.GetOAuth2PermissionScopeIdAsync(tenantId, resourceAppId, scopeName, retryCt), + result => !result.HasValue, cancellationToken: ct); } catch (Exception ex)