Improve agent setup: clarify S2S and consent messaging

CHANGELOG.md

@@ -40,8 +40,11 @@ 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 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.
 - `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

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,22 +672,36 @@ public static async Task<bool> ConfigureBotPermissionsAsync(
         {
             var specs = new List<ResourcePermissionSpec>(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);
 
+            // != 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("");
-            logger.LogInformation("Bot API permissions configured successfully");
+            if (!s2sFailed && consentGranted)
+                logger.LogInformation("Bot API permissions configured successfully");
+            else if (!consentGranted)
+                logger.LogInformation("Bot API permissions configured; admin consent required");
+            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)
         {

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("You are running as a non-admin user and cannot grant admin consent.");
+        _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);
     }
 

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<string>(), Arg.Any<string>(), Arg.Any<CancellationToken>(),
+            Arg.Any<IEnumerable<string>?>(), Arg.Any<bool>())
+            .Returns(Task.FromResult<string?>("resource-sp-id"));
+
+        _mockGraphApiService.IsCurrentUserAdminAsync(
+            Arg.Any<string>(), Arg.Any<CancellationToken>())
+            .Returns(Task.FromResult(RoleCheckResult.HasRole));
+
+        _mockGraphApiService.CreateOrUpdateOauth2PermissionGrantAsync(
+            Arg.Any<string>(), Arg.Any<string>(), Arg.Any<string>(),
+            Arg.Any<IEnumerable<string>>(), Arg.Any<CancellationToken>(), Arg.Any<IEnumerable<string>?>())
+            .Returns(Task.FromResult(true));
+
+        _mockBlueprintService.SetInheritablePermissionsAsync(
+            Arg.Any<string>(), Arg.Any<string>(), Arg.Any<string>(),
+            Arg.Any<IEnumerable<string>>(), Arg.Any<IEnumerable<string>?>(), Arg.Any<CancellationToken>())
+            .Returns(Task.FromResult<(bool ok, bool alreadyExists, string? error)>((true, false, null)));
+
+        _mockBlueprintService.GrantAppRoleAssignmentAsync(
+            Arg.Any<string>(), Arg.Any<string>(), Arg.Any<string>(),
+            Arg.Any<IEnumerable<string>>(), Arg.Any<IEnumerable<string>?>(), Arg.Any<CancellationToken>())
+            .Returns(Task.FromResult(s2sSucceeds));
+
+        _mockGraphApiService.GraphGetAsync(
+            Arg.Any<string>(), Arg.Any<string>(),
+            Arg.Any<CancellationToken>(), Arg.Any<IEnumerable<string>?>())
+            .Returns(callInfo =>
+            {
+                var path = callInfo.ArgAt<string>(1);
+                return path.Contains("/me")
+                    ? Task.FromResult<JsonDocument?>(JsonDocument.Parse("{\"id\":\"mock-user-id\"}"))
+                    : Task.FromResult<JsonDocument?>(null);
+            });
+
+        _mockBlueprintService.VerifyInheritablePermissionsAsync(
+            Arg.Any<string>(), Arg.Any<string>(), Arg.Any<string>(),
+            Arg.Any<CancellationToken>(), Arg.Any<IEnumerable<string>?>())
+            .Returns(Task.FromResult<(bool, string[], string?)>((true, Array.Empty<string>(), null)));
+
+        _mockConfigService.SaveStateAsync(Arg.Any<Agent365Config>())
+            .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<EventId>(),
+            Arg.Is<object>(o => (o.ToString() ?? string.Empty).Contains("S2S app role assignment failed")),
+            Arg.Any<Exception?>(),
+            Arg.Any<Func<object, Exception?, string>>());
+    }
+
+    [Fact]
+    public async Task ConfigureBotPermissionsAsync_WhenGraphAuthFails_DoesNotLogSuccessMessage()
+    {
+        // 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",
+            AgentBlueprintId = "blueprint-app-id"
+        };
+
+        _mockGraphApiService.GraphGetAsync(
+            Arg.Any<string>(), Arg.Any<string>(),
+            Arg.Any<CancellationToken>(), Arg.Any<IEnumerable<string>?>())
+            .Returns(Task.FromResult<JsonDocument?>(null));
+
+        _mockConfigService.SaveStateAsync(Arg.Any<Agent365Config>())
+            .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 Graph auth fails");
+        _mockLogger.DidNotReceive().Log(
+            LogLevel.Information,
+            Arg.Any<EventId>(),
+            Arg.Is<object>(o => (o.ToString() ?? string.Empty).Contains("configured successfully")),
+            Arg.Any<Exception>(),
+            Arg.Any<Func<object, Exception?, string>>());
+    }
+
+    [Fact]
+    public async Task ConfigureBotPermissionsAsync_WhenGraphAuthFails_LogsConsentRequiredMessage()
+    {
+        // 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",
+            AgentBlueprintId = "blueprint-app-id"
+        };
+
+        _mockGraphApiService.GraphGetAsync(
+            Arg.Any<string>(), Arg.Any<string>(),
+            Arg.Any<CancellationToken>(), Arg.Any<IEnumerable<string>?>())
+            .Returns(Task.FromResult<JsonDocument?>(null));
+
+        _mockConfigService.SaveStateAsync(Arg.Any<Agent365Config>())
+            .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 Graph auth fails");
+        _mockLogger.Received().Log(
+            LogLevel.Information,
+            Arg.Any<EventId>(),
+            Arg.Is<object>(o => (o.ToString() ?? string.Empty).Contains("admin consent required")),
+            Arg.Any<Exception>(),
+            Arg.Any<Func<object, Exception?, string>>());
+    }
+
+    [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<EventId>(),
+            Arg.Is<object>(o => (o.ToString() ?? string.Empty).Contains("S2S app role assignment failed")),
+            Arg.Any<Exception>(),
+            Arg.Any<Func<object, Exception?, string>>());
+
+        // Assert — success message must not be logged when S2S failed.
+        _mockLogger.DidNotReceive().Log(
+            LogLevel.Information,
+            Arg.Any<EventId>(),
+            Arg.Is<object>(o => (o.ToString() ?? string.Empty).Contains("configured successfully")),
+            Arg.Any<Exception>(),
+            Arg.Any<Func<object, Exception?, string>>());
+    }
+
+    [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<EventId>(),
+            Arg.Is<object>(o => (o.ToString() ?? string.Empty).Contains("configured successfully")),
+            Arg.Any<Exception?>(),
+            Arg.Any<Func<object, Exception?, string>>());
+    }
+
+    [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
 }