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 dacfc8a1..5f2bfd13 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: 5, baseDelaySeconds: 3); } private sealed record ResolvedInput @@ -90,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; @@ -116,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}'..."); @@ -132,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; @@ -174,8 +181,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, ct); DisplayResults(input, addResponse.Server?.RemoteMCPServerProxyRedirectUri, warnings); } @@ -632,15 +638,32 @@ 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 {RedirectUriCount} redirect URIs on '{AppName}' ({ObjectId}): {RedirectUris}", + publicClientUris.Length, + publicClientsAppName, + publicClientsObjectId, + string.Join(", ", publicClientUris)); + } } 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); } } @@ -728,167 +751,270 @@ 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) + string tenantId, List warnings, CancellationToken ct = default) { + 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 - { - 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)); - await _graphApiService!.UpdateAppRedirectUrisAsync(tenantId, apps.A365AppObjectId, a365Uris); - _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); - warnings.Add(msg); - } + tasks.Add(UpdateA365RedirectUrisAsync(tenantId, apps, a365RedirectUri, concurrentWarnings, ct)); } 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) { + tasks.Add(UpdateRemoteProxyRedirectUrisAsync(tenantId, apps, remoteRedirectUri, concurrentWarnings, ct)); + } + 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(AddRemoteProxyScopePermissionAsync(tenantId, input, apps, concurrentWarnings, ct)); + } + + var ppmiAppClientId = response.Server?.PpmiAppClientId; + Guid? ppmiScopeId = null; + if (!string.IsNullOrWhiteSpace(ppmiAppClientId)) + { + _logger.LogDebug("PPMI app provisioned: {PpmiAppClientId}", ppmiAppClientId); 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}): {RedirectUris}", apps.RemoteProxyAppName, apps.RemoteProxyObjectId, string.Join(", ", remoteUris)); - await _graphApiService!.UpdateAppRedirectUrisAsync(tenantId, apps.RemoteProxyObjectId, remoteUris); - _logger.LogInformation("Updated redirect URIs on '{AppName}'", apps.RemoteProxyAppName); + ppmiScopeId = await _retryHelper.ExecuteWithRetryAsync( + async retryCt => await _graphApiService!.GetOAuth2PermissionScopeIdAsync( + tenantId, ppmiAppClientId, "Tools.ListInvoke.All", retryCt), + result => !result.HasValue, + cancellationToken: ct); } catch (Exception ex) { - var msg = $"Failed to update redirect URIs on Remote Proxy app: {ex.Message}"; - _logger.LogWarning(msg); - warnings.Add(msg); + 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); } } - else if (input.IsEntra) + + if (ppmiScopeId.HasValue) { - var msg = "Remote MCP Proxy redirect URI was not returned by the server. Redirect URI configuration skipped."; - _logger.LogWarning(msg); - warnings.Add(msg); + 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, ct)); + } + } + 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); + concurrentWarnings.Add(msg); } + + await Task.WhenAll(tasks); + + foreach (var w in concurrentWarnings) + warnings.Add(w); } - private async Task ConfigureApiPermissionsAsync( - ResolvedInput input, EntraAppSet apps, AddMcpServerResponse response, - string tenantId, List warnings) + + private async Task UpdateA365RedirectUrisAsync( + string tenantId, EntraAppSet apps, string a365RedirectUri, + System.Collections.Concurrent.ConcurrentBag concurrentWarnings, + CancellationToken ct = default) { - if (input.IsEntra && apps.RemoteProxyObjectId != null && !string.IsNullOrWhiteSpace(input.RemoteScopes)) + try { - 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 retryCt => await _graphApiService!.UpdateAppRedirectUrisAsync(tenantId, apps.A365AppObjectId, a365Uris, retryCt), + result => !result, + cancellationToken: ct); + if (!success) { - 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); - } - } + 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); + } + } - 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); - await _graphApiService.AddRequiredResourceAccessAsync( - tenantId, apps.RemoteProxyObjectId, resourceAppId, remoteScopeId.Value); - } - else - { - var msg = $"Could not find scope '{scopeName}' on resource app {resourceAppId}. API permission not added to RemoteProxy app."; - _logger.LogWarning(msg); - warnings.Add(msg); - } - } - else - { - var msg = $"Could not parse resource app ID and scope from '{input.RemoteScopes}'. Expected format: api://{{appId}}/{{scopeName}}"; - _logger.LogWarning(msg); - warnings.Add(msg); - } + private async Task UpdateRemoteProxyRedirectUrisAsync( + string tenantId, EntraAppSet apps, string remoteRedirectUri, + System.Collections.Concurrent.ConcurrentBag concurrentWarnings, + CancellationToken ct = default) + { + 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 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."; + _logger.LogError(msg); + concurrentWarnings.Add(msg); } - catch (Exception ex) + else { - var msg = $"Failed to add API permissions on RemoteProxy app: {ex.Message}"; - _logger.LogWarning(msg); - warnings.Add(msg); + _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); + } + } - var ppmiAppClientId = response.Server?.PpmiAppClientId; - if (!string.IsNullOrWhiteSpace(ppmiAppClientId)) + private async Task AddRemoteProxyScopePermissionAsync( + string tenantId, ResolvedInput input, EntraAppSet apps, + System.Collections.Concurrent.ConcurrentBag concurrentWarnings, + CancellationToken ct = default) + { + try { - _logger.LogDebug("PPMI app provisioned: {PpmiAppClientId}", ppmiAppClientId); + 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); + } + } - var ppmiScopeId = await _graphApiService!.GetOAuth2PermissionScopeIdAsync( - tenantId, ppmiAppClientId, "Tools.ListInvoke.All"); - if (ppmiScopeId.HasValue) + if (!string.IsNullOrWhiteSpace(resourceAppId) && !string.IsNullOrWhiteSpace(scopeName)) { + _logger.LogDebug("Looking up scope '{ScopeName}' on resource app {ResourceAppId}...", scopeName, resourceAppId); + Guid? remoteScopeId = null; 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); + remoteScopeId = await _retryHelper.ExecuteWithRetryAsync( + async retryCt => await _graphApiService!.GetOAuth2PermissionScopeIdAsync(tenantId, resourceAppId, scopeName, retryCt), + result => !result.HasValue, + cancellationToken: ct); } catch (Exception ex) { - var msg = $"Failed to add PPMI permission on A365 Proxy app: {ex.Message}"; - _logger.LogWarning(msg); - warnings.Add(msg); + 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 (apps.PublicClientsObjectId != null) + if (remoteScopeId.HasValue) { - 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); - } - catch (Exception ex) + _logger.LogDebug("Adding API permission '{ScopeName}' (resource: {ResourceAppId}) on '{AppName}' ({ObjectId})", scopeName, resourceAppId, apps.RemoteProxyAppName, apps.RemoteProxyObjectId); + var success = await _retryHelper.ExecuteWithRetryAsync( + async retryCt => await _graphApiService!.AddRequiredResourceAccessAsync( + tenantId, apps.RemoteProxyObjectId!, resourceAppId, remoteScopeId.Value, retryCt), + result => !result, + cancellationToken: ct); + if (!success) { - var msg = $"Failed to add PPMI permission on Copilot app: {ex.Message}"; - _logger.LogWarning(msg); - warnings.Add(msg); + 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 find 'Tools.ListInvoke.All' scope on PPMI app {ppmiAppClientId}. API permissions not added."; + var msg = $"Could not parse resource app ID and scope from '{input.RemoteScopes}'. Expected format: api://{{appId}}/{{scopeName}}"; _logger.LogWarning(msg); - warnings.Add(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); + } } + private async Task AddPpmiPermissionAsync( + string tenantId, string appObjectId, string appName, + string ppmiAppClientId, Guid ppmiScopeId, + 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 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."; + _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)