From fe2c33b565e7a5b144180a4ab8008e0ce5ee2be8 Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Tue, 5 May 2026 10:03:37 -0700 Subject: [PATCH 1/5] Improve agent setup: clarify S2S and consent messaging - Only log "Bot API permissions configured successfully" if S2S app role assignment succeeds; otherwise, show a warning with retry instructions. - Return value now reflects both consent and S2S assignment success to avoid false positives. - Update consent-required message to clarify that a Global Administrator must grant tenant-wide consent, improving accuracy and user guidance. --- CHANGELOG.md | 2 ++ .../SetupSubcommands/PermissionsSubcommand.cs | 12 ++++++++++-- .../Services/MsalBrowserCredential.cs | 2 +- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index caf715f9..09eaee1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,8 @@ Agents provisioned before this release need `Agent365.Observability.OtelWrite` g - Defensive fallback when the server rejects the new request with a known contract-mismatch signature — the CLI logs `"Automated messaging endpoint registration is not available for this tenant yet. You'll need to configure it manually."` and directs the user to the Teams Developer Portal. Same user-facing path is reused when registration fails because the signed-in user is not a blueprint owner. ### Fixed +- `setup permissions bot` no longer emits "Bot API permissions configured successfully" when the Observability API S2S app-role assignment fails; shows a warning with retry instructions instead. +- Consent-required message "You are running as a non-admin user and cannot grant admin consent" replaced with "A Global Administrator must grant tenant-wide consent to proceed" — the message fires when tenant-wide consent for S2S scopes has not yet been granted, not when the caller lacks admin rights. - `setup all --agent-name` re-runs no longer create a duplicate agent registration: the CLI now reads `agentRegistrationId` from `a365.generated.config.json` (when present) and checks for an existing registration before posting a new one. - `setup all` now skips agent registration with a clear warning when the agent identity ID is not available, instead of silently sending an invalid request. Retry with `a365 setup all --agent-registration-only` once the identity is ready. - `setup all --agent-registration-only` reliability fixes: stored IDs are now correctly read in bootstrap (`--agent-name`) mode; falls back to a Graph API lookup when `agenticAppId` is missing; skips identity, permission, and project-settings steps that don't apply. diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs index bed3ed86..9c660350 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs @@ -678,14 +678,22 @@ public static async Task ConfigureBotPermissionsAsync( await configService.SaveStateAsync(setupConfig); + var s2sFailed = setupResults?.S2SAppRoleGranted == false; + logger.LogInformation(""); - logger.LogInformation("Bot API permissions configured successfully"); + if (!s2sFailed) + logger.LogInformation("Bot API permissions configured successfully"); + else + logger.LogWarning( + "Bot API permissions configured, but S2S app role assignment failed. " + + "Re-run 'a365 setup permissions bot' as {Roles} to retry.", + AuthenticationConstants.S2SGrantRequiredRoles); logger.LogInformation(""); if (!iSetupAll) { logger.LogInformation("Next step: Deploy your agent (run 'a365 deploy' if hosting on Azure)"); } - return consentGranted; + return consentGranted && !s2sFailed; } catch (Exception ex) { diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs index 7ba3d456..7637d573 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs @@ -496,7 +496,7 @@ private void LogConsentRequiredAndThrow(Exception inner) { var consentUrl = ClientAppValidationException.BuildAdminConsentUrl(_clientAppId, _tenantId); _logger?.LogWarning("Admin consent has not been granted for this application."); - _logger?.LogWarning("You are running as a non-admin user and cannot grant admin consent."); + _logger?.LogWarning("A Global Administrator must grant tenant-wide consent to proceed."); if (consentUrl != null) { _logger?.LogWarning("Share this URL with a Global Administrator to grant consent:"); From b7460a415a17176747acec4a88a6e708cebc8446 Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Tue, 5 May 2026 12:44:07 -0700 Subject: [PATCH 2/5] Address Copilot review comments on PR #404 --- CHANGELOG.md | 1 + .../SetupSubcommands/PermissionsSubcommand.cs | 13 +- .../Commands/PermissionsSubcommandTests.cs | 260 ++++++++++++++++++ 3 files changed, 270 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 09eaee1b..252063d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,7 @@ Agents provisioned before this release need `Agent365.Observability.OtelWrite` g - Consent-required message "You are running as a non-admin user and cannot grant admin consent" replaced with "A Global Administrator must grant tenant-wide consent to proceed" — the message fires when tenant-wide consent for S2S scopes has not yet been granted, not when the caller lacks admin rights. - `setup all --agent-name` re-runs no longer create a duplicate agent registration: the CLI now reads `agentRegistrationId` from `a365.generated.config.json` (when present) and checks for an existing registration before posting a new one. - `setup all` now skips agent registration with a clear warning when the agent identity ID is not available, instead of silently sending an invalid request. Retry with `a365 setup all --agent-registration-only` once the identity is ready. +- `setup permissions bot` now returns a non-zero exit code when an S2S app role assignment fails, so callers and scripts can detect the failure. - `setup all --agent-registration-only` reliability fixes: stored IDs are now correctly read in bootstrap (`--agent-name`) mode; falls back to a Graph API lookup when `agenticAppId` is missing; skips identity, permission, and project-settings steps that don't apply. ### Removed diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs index 9c660350..081acc97 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs @@ -324,7 +324,7 @@ private static Command CreateBotSubcommand( var botChecks = GetBotChecks(authValidator); await RequirementsSubcommand.RunChecksOrExitAsync(botChecks, setupConfig, logger, ct); - await ConfigureBotPermissionsAsync( + var success = await ConfigureBotPermissionsAsync( configFile.FullName, logger, configService, @@ -333,6 +333,8 @@ await ConfigureBotPermissionsAsync( graphApiService, blueprintService, false); + if (!success) + context.ExitCode = 1; }); @@ -670,19 +672,22 @@ public static async Task ConfigureBotPermissionsAsync( { var specs = new List(SetupHelpers.GetFixedApiPermissionSpecs(setInheritable: true)); + var localResults = setupResults ?? new SetupResults(); var (_, _, consentGranted, _) = await BatchPermissionsOrchestrator.ConfigureAllPermissionsAsync( graphService, blueprintService, setupConfig, setupConfig.AgentBlueprintId!, setupConfig.TenantId, - specs, logger, setupResults, cancellationToken, + specs, logger, localResults, cancellationToken, knownBlueprintSpObjectId: setupConfig.AgentBlueprintServicePrincipalObjectId); await configService.SaveStateAsync(setupConfig); - var s2sFailed = setupResults?.S2SAppRoleGranted == false; + var s2sFailed = localResults.S2SAppRoleGranted == false; logger.LogInformation(""); - if (!s2sFailed) + if (!s2sFailed && consentGranted) logger.LogInformation("Bot API permissions configured successfully"); + else if (!s2sFailed) + logger.LogInformation("Bot API permissions configured; admin consent required"); else logger.LogWarning( "Bot API permissions configured, but S2S app role assignment failed. " + diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PermissionsSubcommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PermissionsSubcommandTests.cs index f2aa3e32..3f2d4cf7 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PermissionsSubcommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PermissionsSubcommandTests.cs @@ -9,6 +9,7 @@ using Microsoft.Extensions.Logging.Abstractions; using NSubstitute; using System.CommandLine; +using System.Text.Json; using Xunit; namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Commands; @@ -741,5 +742,264 @@ public async Task ConfigureCustomPermissionsAsync_WithEmptyList_SkipsGracefully( // - SetupResults tracking for custom permissions #endregion + + #region ConfigureBotPermissionsAsync — Issue 402 Regression Tests + // Regression tests for the localResults fix: S2S failure was invisible when setupResults=null. + + private Agent365Config ArrangeAdminPath(bool s2sSucceeds) + { + var config = new Agent365Config + { + TenantId = "00000000-0000-0000-0000-000000000000", + AgentBlueprintId = "blueprint-app-id", + AgentBlueprintServicePrincipalObjectId = "blueprint-sp-id" + }; + + _mockGraphApiService.EnsureServicePrincipalForAppIdAsync( + Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any?>(), Arg.Any()) + .Returns(Task.FromResult("resource-sp-id")); + + _mockGraphApiService.IsCurrentUserAdminAsync( + Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(RoleCheckResult.HasRole)); + + _mockGraphApiService.CreateOrUpdateOauth2PermissionGrantAsync( + Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any>(), Arg.Any(), Arg.Any?>()) + .Returns(Task.FromResult(true)); + + _mockBlueprintService.SetInheritablePermissionsAsync( + Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any>(), Arg.Any?>(), Arg.Any()) + .Returns(Task.FromResult<(bool ok, bool alreadyExists, string? error)>((true, false, null))); + + _mockBlueprintService.GrantAppRoleAssignmentAsync( + Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any>(), Arg.Any?>(), Arg.Any()) + .Returns(Task.FromResult(s2sSucceeds)); + + _mockGraphApiService.GraphGetAsync( + Arg.Any(), Arg.Any(), + Arg.Any(), Arg.Any?>()) + .Returns(callInfo => + { + var path = callInfo.ArgAt(1); + return path.Contains("/me") + ? Task.FromResult(JsonDocument.Parse("{\"id\":\"mock-user-id\"}")) + : Task.FromResult(null); + }); + + _mockBlueprintService.VerifyInheritablePermissionsAsync( + Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any(), Arg.Any?>()) + .Returns(Task.FromResult<(bool, string[], string?)>((true, Array.Empty(), null))); + + _mockConfigService.SaveStateAsync(Arg.Any()) + .Returns(Task.CompletedTask); + + return config; + } + + [Fact] + public async Task ConfigureBotPermissionsAsync_AdminPath_WhenS2SFailsAndNoExternalSetupResults_ReturnsFalse() + { + // Arrange — admin user, OAuth2 grants succeed, S2S grant fails, no external SetupResults. + var config = ArrangeAdminPath(s2sSucceeds: false); + + // Act — no external SetupResults, mirrors how the CLI handler calls this method. + var result = await PermissionsSubcommand.ConfigureBotPermissionsAsync( + "config.json", + _mockLogger, + _mockConfigService, + _mockExecutor, + config, + _mockGraphApiService, + _mockBlueprintService, + iSetupAll: false, + setupResults: null); + + // Assert + result.Should().BeFalse( + because: "the method must create a local SetupResults so S2S failure is captured " + + "even when called without an external setupResults (the CLI handler path)"); + _mockLogger.Received().Log( + LogLevel.Warning, + Arg.Any(), + Arg.Is(o => (o.ToString() ?? string.Empty).Contains("S2S app role assignment failed")), + Arg.Any(), + Arg.Any>()); + } + + [Fact] + public async Task ConfigureBotPermissionsAsync_WhenConsentPending_DoesNotLogSuccessMessage() + { + // Arrange — Graph returns null → consentGranted=false; S2S not attempted. + var config = new Agent365Config + { + TenantId = "00000000-0000-0000-0000-000000000000", + AgentBlueprintId = "blueprint-app-id" + }; + + _mockGraphApiService.GraphGetAsync( + Arg.Any(), Arg.Any(), + Arg.Any(), Arg.Any?>()) + .Returns(Task.FromResult(null)); + + _mockConfigService.SaveStateAsync(Arg.Any()) + .Returns(Task.CompletedTask); + + // Act + var result = await PermissionsSubcommand.ConfigureBotPermissionsAsync( + "config.json", + _mockLogger, + _mockConfigService, + _mockExecutor, + config, + _mockGraphApiService, + _mockBlueprintService, + iSetupAll: false, + setupResults: null); + + // Assert + result.Should().BeFalse(because: "consentGranted=false when no admin user consents"); + _mockLogger.DidNotReceive().Log( + LogLevel.Information, + Arg.Any(), + Arg.Is(o => (o.ToString() ?? string.Empty).Contains("configured successfully")), + Arg.Any(), + Arg.Any>()); + } + + [Fact] + public async Task ConfigureBotPermissionsAsync_WhenConsentPending_LogsConsentRequiredMessage() + { + // Arrange — same as above: Graph returns null → consentGranted=false, S2S not attempted. + var config = new Agent365Config + { + TenantId = "00000000-0000-0000-0000-000000000000", + AgentBlueprintId = "blueprint-app-id" + }; + + _mockGraphApiService.GraphGetAsync( + Arg.Any(), Arg.Any(), + Arg.Any(), Arg.Any?>()) + .Returns(Task.FromResult(null)); + + _mockConfigService.SaveStateAsync(Arg.Any()) + .Returns(Task.CompletedTask); + + // Act + var result = await PermissionsSubcommand.ConfigureBotPermissionsAsync( + "config.json", + _mockLogger, + _mockConfigService, + _mockExecutor, + config, + _mockGraphApiService, + _mockBlueprintService, + iSetupAll: false, + setupResults: null); + + // Assert + result.Should().BeFalse(because: "consentGranted=false when no admin user consents"); + _mockLogger.Received().Log( + LogLevel.Information, + Arg.Any(), + Arg.Is(o => (o.ToString() ?? string.Empty).Contains("admin consent required")), + Arg.Any(), + Arg.Any>()); + } + + [Fact] + public async Task ConfigureBotPermissionsAsync_AdminPath_WhenS2SFails_EmitsS2SWarningAndSuppressesSuccessLog() + { + // Arrange — admin user, OAuth2 grants succeed, S2S grant fails. + var config = ArrangeAdminPath(s2sSucceeds: false); + + // Act + _ = await PermissionsSubcommand.ConfigureBotPermissionsAsync( + "config.json", + _mockLogger, + _mockConfigService, + _mockExecutor, + config, + _mockGraphApiService, + _mockBlueprintService, + iSetupAll: false, + setupResults: null); + + // Assert — S2S failure warning must be logged. + _mockLogger.Received().Log( + LogLevel.Warning, + Arg.Any(), + Arg.Is(o => (o.ToString() ?? string.Empty).Contains("S2S app role assignment failed")), + Arg.Any(), + Arg.Any>()); + + // Assert — success message must not be logged when S2S failed. + _mockLogger.DidNotReceive().Log( + LogLevel.Information, + Arg.Any(), + Arg.Is(o => (o.ToString() ?? string.Empty).Contains("configured successfully")), + Arg.Any(), + Arg.Any>()); + } + + [Fact] + public async Task ConfigureBotPermissionsAsync_AdminPath_WhenAllSucceeds_ReturnsTrueAndLogsSuccess() + { + // Arrange — admin user, all OAuth2 grants succeed, S2S grant succeeds. + var config = ArrangeAdminPath(s2sSucceeds: true); + + // Act + var result = await PermissionsSubcommand.ConfigureBotPermissionsAsync( + "config.json", + _mockLogger, + _mockConfigService, + _mockExecutor, + config, + _mockGraphApiService, + _mockBlueprintService, + iSetupAll: false, + setupResults: null); + + // Assert + result.Should().BeTrue(because: "all grants and S2S succeed — method must return true"); + _mockLogger.Received().Log( + LogLevel.Information, + Arg.Any(), + Arg.Is(o => (o.ToString() ?? string.Empty).Contains("configured successfully")), + Arg.Any(), + Arg.Any>()); + } + + [Fact] + public async Task ConfigureBotPermissionsAsync_AdminPath_WhenS2SFailsWithExternalSetupResults_PopulatesS2SFlagAndReturnsFalse() + { + // Arrange — S2S fails, non-null SetupResults passed in (AllSubcommand path). + var config = ArrangeAdminPath(s2sSucceeds: false); + var externalResults = new SetupResults(); + + // Act + var result = await PermissionsSubcommand.ConfigureBotPermissionsAsync( + "config.json", + _mockLogger, + _mockConfigService, + _mockExecutor, + config, + _mockGraphApiService, + _mockBlueprintService, + iSetupAll: false, + setupResults: externalResults); + + // Assert + result.Should().BeFalse( + because: "S2S failure must propagate via the external SetupResults reference"); + externalResults.S2SAppRoleGranted.Should().BeFalse( + because: "the orchestrator must write S2SAppRoleGranted=false to the caller's SetupResults"); + } + + #endregion } From 91d373e37d16302e2f117c01d94f3e28aa07a4fb Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Tue, 5 May 2026 13:10:17 -0700 Subject: [PATCH 3/5] Address Copilot review comment: broaden consent message from Global Administrator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Application Administrator and Agent ID Administrator can also grant tenant-wide admin consent — the previous wording incorrectly implied only Global Administrators could do so. Co-Authored-By: Claude Sonnet 4.6 --- .../Services/MsalBrowserCredential.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs index 7637d573..81362e07 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs @@ -496,17 +496,17 @@ private void LogConsentRequiredAndThrow(Exception inner) { var consentUrl = ClientAppValidationException.BuildAdminConsentUrl(_clientAppId, _tenantId); _logger?.LogWarning("Admin consent has not been granted for this application."); - _logger?.LogWarning("A Global Administrator must grant tenant-wide consent to proceed."); + _logger?.LogWarning("An administrator must grant tenant-wide consent to proceed."); if (consentUrl != null) { - _logger?.LogWarning("Share this URL with a Global Administrator to grant consent:"); + _logger?.LogWarning("Share this URL with an administrator to grant consent:"); _logger?.LogWarning(" {ConsentUrl}", consentUrl); } _logger?.LogWarning("After consent is granted, re-run the command."); throw new MsalAuthenticationFailedException( consentUrl != null - ? $"Admin consent required. Share this URL with a Global Administrator: {consentUrl}" - : "Admin consent required. A Global Administrator must grant tenant-wide consent for this application.", + ? $"Admin consent required. Share this URL with an administrator: {consentUrl}" + : "Admin consent required. An administrator must grant tenant-wide consent for this application.", inner); } From 4c2cc5c28a7644f95915abb333adc27313aa5a0b Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Tue, 5 May 2026 13:54:59 -0700 Subject: [PATCH 4/5] Address Copilot review comment: use != true for S2SAppRoleGranted null safety Reorders the logging conditional so the non-admin path (consentGranted=false) is checked before the S2S state, preventing a null S2SAppRoleGranted from routing to the S2S-failure warning instead of the consent-required message. Co-Authored-By: Claude Sonnet 4.6 --- .../Commands/SetupSubcommands/PermissionsSubcommand.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs index 081acc97..d267a8b9 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs @@ -681,12 +681,15 @@ public static async Task ConfigureBotPermissionsAsync( await configService.SaveStateAsync(setupConfig); - var s2sFailed = localResults.S2SAppRoleGranted == false; + // != true treats both explicit failure (false) and skipped-due-to-error (null) as failure. + // Consent state is checked before S2S so the non-admin path still shows "consent required" + // rather than the S2S warning (S2S is never attempted when consent isn't granted). + var s2sFailed = localResults.S2SAppRoleGranted != true; logger.LogInformation(""); if (!s2sFailed && consentGranted) logger.LogInformation("Bot API permissions configured successfully"); - else if (!s2sFailed) + else if (!consentGranted) logger.LogInformation("Bot API permissions configured; admin consent required"); else logger.LogWarning( From ecdb4552117761d420c20d1aa3aaf7ab7229f95a Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Tue, 5 May 2026 14:25:04 -0700 Subject: [PATCH 5/5] Address Copilot review: fix CHANGELOG wording and rename misnamed tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - CHANGELOG: broaden S2S entry from Observability-specific to any S2S failure - CHANGELOG: correct consent message to match actual code ("An administrator" not "A Global Administrator") - Rename WhenConsentPending tests to WhenGraphAuthFails — null GraphGetAsync drives the phase1Result=null (auth-failed) path, not the non-admin consent-pending path Co-Authored-By: Claude Sonnet 4.6 --- CHANGELOG.md | 4 ++-- .../Commands/PermissionsSubcommandTests.cs | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 252063d5..99c9a43d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,8 +40,8 @@ Agents provisioned before this release need `Agent365.Observability.OtelWrite` g - Defensive fallback when the server rejects the new request with a known contract-mismatch signature — the CLI logs `"Automated messaging endpoint registration is not available for this tenant yet. You'll need to configure it manually."` and directs the user to the Teams Developer Portal. Same user-facing path is reused when registration fails because the signed-in user is not a blueprint owner. ### Fixed -- `setup permissions bot` no longer emits "Bot API permissions configured successfully" when the Observability API S2S app-role assignment fails; shows a warning with retry instructions instead. -- Consent-required message "You are running as a non-admin user and cannot grant admin consent" replaced with "A Global Administrator must grant tenant-wide consent to proceed" — the message fires when tenant-wide consent for S2S scopes has not yet been granted, not when the caller lacks admin rights. +- `setup permissions bot` no longer emits "Bot API permissions configured successfully" when any S2S app-role assignment fails; shows a warning with retry instructions instead. +- Consent-required message "You are running as a non-admin user and cannot grant admin consent" replaced with "An administrator must grant tenant-wide consent to proceed" — the message fires when tenant-wide consent for S2S scopes has not yet been granted, not when the caller lacks admin rights. - `setup all --agent-name` re-runs no longer create a duplicate agent registration: the CLI now reads `agentRegistrationId` from `a365.generated.config.json` (when present) and checks for an existing registration before posting a new one. - `setup all` now skips agent registration with a clear warning when the agent identity ID is not available, instead of silently sending an invalid request. Retry with `a365 setup all --agent-registration-only` once the identity is ready. - `setup permissions bot` now returns a non-zero exit code when an S2S app role assignment fails, so callers and scripts can detect the failure. diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PermissionsSubcommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PermissionsSubcommandTests.cs index 3f2d4cf7..102710c7 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PermissionsSubcommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PermissionsSubcommandTests.cs @@ -832,9 +832,9 @@ public async Task ConfigureBotPermissionsAsync_AdminPath_WhenS2SFailsAndNoExtern } [Fact] - public async Task ConfigureBotPermissionsAsync_WhenConsentPending_DoesNotLogSuccessMessage() + public async Task ConfigureBotPermissionsAsync_WhenGraphAuthFails_DoesNotLogSuccessMessage() { - // Arrange — Graph returns null → consentGranted=false; S2S not attempted. + // Arrange — all Graph calls return null → phase1Result=null (auth failed); consentGranted=false; S2S not attempted. var config = new Agent365Config { TenantId = "00000000-0000-0000-0000-000000000000", @@ -862,7 +862,7 @@ public async Task ConfigureBotPermissionsAsync_WhenConsentPending_DoesNotLogSucc setupResults: null); // Assert - result.Should().BeFalse(because: "consentGranted=false when no admin user consents"); + result.Should().BeFalse(because: "consentGranted=false when Graph auth fails"); _mockLogger.DidNotReceive().Log( LogLevel.Information, Arg.Any(), @@ -872,9 +872,9 @@ public async Task ConfigureBotPermissionsAsync_WhenConsentPending_DoesNotLogSucc } [Fact] - public async Task ConfigureBotPermissionsAsync_WhenConsentPending_LogsConsentRequiredMessage() + public async Task ConfigureBotPermissionsAsync_WhenGraphAuthFails_LogsConsentRequiredMessage() { - // Arrange — same as above: Graph returns null → consentGranted=false, S2S not attempted. + // Arrange — all Graph calls return null → phase1Result=null (auth failed); consentGranted=false, S2S not attempted. var config = new Agent365Config { TenantId = "00000000-0000-0000-0000-000000000000", @@ -902,7 +902,7 @@ public async Task ConfigureBotPermissionsAsync_WhenConsentPending_LogsConsentReq setupResults: null); // Assert - result.Should().BeFalse(because: "consentGranted=false when no admin user consents"); + result.Should().BeFalse(because: "consentGranted=false when Graph auth fails"); _mockLogger.Received().Log( LogLevel.Information, Arg.Any(),