Reduce custom app permissions and improve setup reliability

.claude/agents/pr-code-reviewer.md

@@ -215,6 +215,56 @@ For each changed file, analyze:
    - **Fix pattern**: replace `context.ExitCode = 1;` with `context.ExitCode = ex.ExitCode;` so the exit code is always authoritative from the exception definition
    - **Real example (PR #406, Comments 5 & 6)**: `ConfigFileNotFoundException.ExitCode => 2`, but two catch blocks in `AllSubcommand.cs` hardcoded `context.ExitCode = 1`. Scripts expecting exit code 2 on configuration errors received 1 instead.
 
+   **E. Static scope array emptied or scopes removed → verify callers and document intent**
+   When the diff sets a static `string[]` scope array to `[]` or removes entries from it (e.g., `RequiredPermissionGrantScopes`, `RequiredS2SGrantScopes`):
+   - Run `Grep` for every reference to the array across non-test source files
+   - For each call site that passes the array as `scopes` to `EnsureGraphHeadersAsync` or equivalent:
+     - Read the `hasScopes = scopes?.Any() == true` guard logic — confirm the empty-array fallback path still acquires a valid token via `GetGraphAccessTokenAsync` (the standard `AuthenticationService` path)
+     - Confirm the operations that previously needed those scopes are still covered by `RequiredClientAppPermissions` or by a PowerShell fallback
+   - Check that the XML doc on the constant explains WHY it is empty and names the fallback behavior
+   - Severity: **MEDIUM** if XML doc doesn't explain empty state; **HIGH** if fallback path would fail in practice
+
+   **F. Runtime `Contains()` on a static compile-time array → verify element IS in the array**
+   When the diff adds `SomeStaticArray.Contains(SomeConstant)` as a guard condition:
+   - Read `SomeStaticArray`'s initializer — confirm whether `SomeConstant` is actually present
+   - If `SomeConstant` is intentionally absent (disabled feature, future path), the condition is always `false`:
+     - Verify the `else` branch handles the always-false case correctly and critical functionality is not silently skipped
+     - Require a comment that makes the always-false state explicit AND states the consequence (e.g., "this means agent user cleanup is always skipped until create-instance is re-enabled")
+   - Severity: **MEDIUM** if consequence is limited scope; **HIGH** if critical functionality (e.g., cleanup of real resources) is silently disabled with no user-visible warning
+   - **Real example (PR #409)**: `RequiredClientAppPermissions.Contains(AgentIdUserReadWriteAllScope)` is always `false` because that scope is intentionally absent — agent user queries and cleanup always skip silently
+
+   **G. XML/inline comments claiming permission scope semantics → verify against actual scope requirements**
+   When the diff adds or modifies `///` XML comments (especially in test files) that describe what a permission scope covers (e.g., "umbrella — includes SP creation", "covers X, Y, Z"):
+   - Cross-check the claim against `RequiredClientAppPermissions` and the PR's own changes
+   - Specifically: if the PR adds a *separate* scope for an operation (e.g., `AgentIdentityBlueprintPrincipal.Create` for SP creation), any comment claiming that operation is "covered by the umbrella" is now wrong
+   - Flag **MEDIUM** for inaccurate scope coverage claims in comments — these mislead future reviewers and generate follow-up PR comments
+   - **Real example (PR #409)**: XML comment said `AgentIdentityBlueprint.ReadWrite.All` "includes SP creation" but the same PR requires `AgentIdentityBlueprintPrincipal.Create` separately for SP creation
+
+   **H. JWT claim decoded → verify the token was issued by the app registration that has the claim configured**
+   When the diff adds code that decodes a JWT and reads a claim that is configured *per app registration*
+   (e.g., `wids`, `roles`, `groups`, any custom optional claim from Entra Token Configuration):
+   - Trace backward from the decoder to the token-acquisition call. Identify which clientId issues the token.
+   - If acquisition goes through `AuthenticationService.GetAccessTokenAsync(...)` without an explicit
+     `clientId` argument, the token is from the PowerShell well-known clientId
+     (`AuthenticationConstants.PowershellClientId`) — **not** the custom client app. Optional claims
+     configured on the custom client app's manifest will NOT be in this token.
+   - The correct path for tokens that must carry custom-app optional claims is
+     `IMicrosoftGraphTokenProvider.GetMgGraphAccessTokenAsync(..., clientAppId: CustomClientAppId, ...)`.
+   - Flag **HIGH** when the decoder relies on a claim that the issuing app doesn't have configured.
+     The bug is silent: the decoder returns "claim absent → Unknown / DoesNotHaveRole", and the caller
+     proceeds with the conservative path (e.g., admin treated as non-admin → admin URL printed even
+     for actual admins). Functional but degraded UX, easy to miss in passing tests because mocks
+     usually return a hand-crafted JWT regardless of which auth path is used.
+   - Also apply this rule to **empty static scope arrays** (related to Rule E): when an empty
+     `IEnumerable<string>` is passed to a helper that routes "no scopes" through the legacy
+     `AuthenticationService` path, the token will be from the PowerShell clientId. If any downstream
+     code reads custom-app optional claims, the routing is wrong.
+   - **Real example (PR #409)**: `CheckDirectoryRoleAsync` decoded `wids` from a token acquired via
+     `GetGraphAccessTokenAsync` → `AuthenticationService` → PowerShell clientId. `wids` was
+     configured on the custom client app only, so the JWT never carried it and the method always
+     returned `Unknown`. Fix: route through `_tokenProvider` with `CustomClientAppId` and a minimal
+     scope (`User.Read`).
+
    **D2. Cross-method "catch-returns-null / caller-sets-ExitCode" pattern — same exit-code problem, harder to spot**
    Rule 9-D catches *inline* catch blocks. This variant spans two methods and is easy to miss:
    ```csharp

.github/workflows/ci.yml

@@ -13,6 +13,7 @@ jobs:
   dotnet-sdk:
     name: .NET SDK
     runs-on: ubuntu-latest
+    timeout-minutes: 20
     permissions:
       contents: write
     defaults:
@@ -52,7 +53,7 @@ jobs:
         run: dotnet build tests.proj --no-restore --configuration Release
 
       - name: Run tests
-        run: dotnet test tests.proj --no-build --configuration Release --verbosity normal
+        run: dotnet test tests.proj --no-build --configuration Release --verbosity normal --blame-hang --blame-hang-timeout 5min
 
       - name: Pack NuGet packages
         id: pack

.gitignore

@@ -1,5 +1,6 @@
 # Internal working documents
 docs/plans/
+docs/min-permissions/
 docs/Permissions-Review.md
 docs/Testing.md
 scripts/skills/

CHANGELOG.md

@@ -41,7 +41,14 @@ Agents provisioned before this release need `Agent365.Observability.OtelWrite` g
 - Messaging endpoint row added to `a365 setup all` summary output, with "registered"/"reused"/"skipped (non-M365)"/"manual config required"/"failed" states. When registration can't complete, the summary surfaces an "Action Required" entry with the Teams Developer Portal URL so the user knows exactly what to do next.
 - 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.
 
+### Changed
+- Agent identity creation now uses Blueprint app-only credentials (`AgentIdentity.CreateAsManager`, auto-granted to all Blueprint apps). The custom CLI app no longer requires `AgentIdentity.Create.All` or `DelegatedPermissionGrant.ReadWrite.All`. Administrators can remove these permissions from the CLI app registration. See [Custom client app registration](https://learn.microsoft.com/microsoft-agent-365/developer/custom-client-app-registration) for the updated permission list.
+- `setup all` now retries agent identity creation and blueprint token acquisition with exponential back-off (delay doubles up to a 60-second cap; agent identity retries up to 5 times, blueprint token up to 12 times — worst case is several minutes per call when Entra replication lag is severe) when Entra replication lag causes transient 401/AADSTS errors on fresh blueprint setups. Retry progress is logged at `Debug` level only.
+
 ### Fixed
+- `setup all` with `--authmode obo --aiteammate` no longer exits with an error. `obo` is the default for AI Teammate agents and is accepted with a warning; `--authmode s2s` and `--authmode both` remain incompatible with `--aiteammate`.
+- `setup all` summary now shows **Messaging endpoint — not configured** (with a link to the Teams Developer Portal to register the endpoint manually) instead of **failed — see Action Required** when the messaging endpoint URL is absent from config. Real failures (bad URL, contract mismatch) continue to show the error path.
+- `setup blueprint` no longer silently loses the blueprint client secret on re-runs when `agentBlueprintClientSecret` was previously `null` in `a365.generated.config.json`. Null dynamic properties are now omitted from the generated config file instead of being written as explicit nulls (fixes macOS secret-loss regression introduced in the issue #408 fix).
 - Error messages for commands run without required configuration no longer expose internal file paths. `setup all`, `cleanup`, and `create-instance` without `--agent-name` now show actionable guidance with the exact command to run. `develop addpermissions` and `develop gettoken` without `--app-id` now prompt for the application ID directly.
 - `setup all` no longer inherits stale resource IDs when the user switches tenants between runs (`az logout` + `az login` to a different tenant). The CLI detects the tenant change before loading configuration, silently backs up files from the previous run, and prompts the user to re-run with `--agent-name` for a clean setup in the new tenant.
 - `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.
@@ -50,6 +57,7 @@ Agents provisioned before this release need `Agent365.Observability.OtelWrite` g
 - `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.
+- `setup permissions bot` help text and final "Next step" log no longer suggest the non-existent `a365 deploy` command; both now point at `a365 publish` (the actual next command in the workflow).
 
 ### Removed
 - `a365 config` command family (`config init`, `config display`, `config permissions`) — replaced by `a365 setup all --agent-name` and `a365 setup permissions custom`.

src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs

@@ -179,9 +179,16 @@ public static Command CreateCommand(
             }
             if (authMode is not null && aiTeammateFlag == true)
             {
-                logger.LogError("--authmode is not supported with --aiteammate — AI Teammate agents automatically use OBO via agent user identity.");
-                context.ExitCode = 1;
-                return;
+                if (authMode == "obo")
+                {
+                    logger.LogWarning("--authmode obo is redundant with --aiteammate — AI Teammate agents always use OBO. Flag ignored.");
+                }
+                else
+                {
+                    logger.LogError("--authmode {AuthMode} is not supported with --aiteammate — AI Teammate agents always use OBO via agent user identity.", authMode);
+                    context.ExitCode = 1;
+                    return;
+                }
             }
 
             // Generate correlation ID at workflow entry point
@@ -782,7 +789,7 @@ internal static async Task ExecuteMessagingEndpointStepAsync(SetupContext ctx)
         {
             ctx.Logger.LogWarning("Messaging endpoint registration skipped: agent blueprint ID is missing (the blueprint step likely failed).");
             ctx.Results.MessagingEndpointResult = Models.EndpointRegistrationResult.Failed;
-            ctx.Results.MessagingEndpointFailureReason = "BlueprintMissing";
+            ctx.Results.MessagingEndpointFailureReason = MessagingEndpointFailureReasons.BlueprintMissing;
             ctx.Results.MessagingEndpoint = ctx.Config.MessagingEndpoint;
             ctx.Results.Warnings.Add("Messaging endpoint: agent blueprint ID is missing, so endpoint registration was not attempted. Resolve the blueprint creation failure first, then re-run 'a365 setup blueprint --endpoint-only --m365'.");
             return;
@@ -816,7 +823,7 @@ internal static async Task ExecuteMessagingEndpointStepAsync(SetupContext ctx)
             // setup should continue; surface the failure in the summary as a warning.
             ctx.Logger.LogWarning("Messaging endpoint registration skipped: {Message}", ex.Message);
             ctx.Results.MessagingEndpointResult = Models.EndpointRegistrationResult.Failed;
-            ctx.Results.MessagingEndpointFailureReason = "Other";
+            ctx.Results.MessagingEndpointFailureReason = MessagingEndpointFailureReasons.Other;
             ctx.Results.MessagingEndpoint = ctx.Config.MessagingEndpoint;
             ctx.Results.Warnings.Add($"Messaging endpoint: {ex.Message}");
         }

src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs

@@ -790,7 +790,7 @@ public static async Task<bool> EnsureDelegatedConsentWithRetriesAsync(
         {
             logger.LogError(ex, "Error during delegated consent: {Message}", ex.Message);
             logger.LogError("Common causes:");
-            logger.LogError("  1. Insufficient permissions - You need Application.ReadWrite.All and DelegatedPermissionGrant.ReadWrite.All");
+            logger.LogError("  1. Insufficient permissions - You need AgentIdentityBlueprint.ReadWrite.All consented on your client app");
             logger.LogError("  2. Not a Global Administrator or similar privileged role");
             logger.LogError("  3. Azure CLI authentication expired - Run 'az login' and retry");
             logger.LogError("  4. Network connectivity issues");
@@ -1021,7 +1021,9 @@ public static async Task<bool> EnsureDelegatedConsentWithRetriesAsync(
             // Explicit scopes — NOT .default. Using .default bundles all consented scopes including
             // AgentIdentityBlueprint.*, which Entra rejects for POST /v1.0/servicePrincipals
             // ("backing application must be in the local tenant").
-            // AgentIdentityBlueprintPrincipal.Create is the correct scope per Agent ID team (Kyle Marsh).
+            // AgentIdentityBlueprintPrincipal.Create is required for POST /v1.0/serviceprincipals/graph.agentIdentityBlueprintPrincipal.
+            // AgentIdentityBlueprint.ReadWrite.All is the umbrella for blueprint app operations but does NOT cover
+            // blueprint SP creation — that is a separate resource requiring Principal.Create (confirmed Run 1, 2026-05-08).
             logger.LogDebug("Acquiring blueprint httpClient token — scope: AgentIdentityBlueprintPrincipal.Create, loginHint: {LoginHint}", blueprintLoginHint ?? "(none)");
             var graphToken = await AcquireMsalGraphTokenAsync(tenantId, setupConfig.ClientAppId, logger, ct,
                 scope: AuthenticationConstants.AgentIdentityBlueprintPrincipalCreateScope,
@@ -1512,7 +1514,7 @@ await retryHelper.ExecuteWithRetryAsync(
                 ficError = ficCreateResult?.ErrorMessage
                     ?? "Federated Identity Credential creation failed";
                 logger.LogWarning("[WARN] Federated Identity Credential creation failed - you may need to create it manually in Entra ID");
-                logger.LogWarning("  Ensure the client app has 'AgentIdentityBlueprint.UpdateAuthProperties.All' permission consented.");
+                logger.LogWarning("  Ensure the client app has 'AgentIdentityBlueprint.ReadWrite.All' permission consented.");
             }
         }
         else if (!useManagedIdentity)

src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/NonDwBlueprintSetupOrchestrator.cs

@@ -428,12 +428,24 @@ private static async Task ExecuteAgentIdentityAndRegistrationAsync(
                 }
                 else
                 {
-                    // Agent identity creation via delegated flow (AgentIdentity.Create.All).
-                    // Agent ID Developer role is sufficient — client credentials are not required.
+                    // Agent identity creation via blueprint client credentials (app-only).
+                    // AgentIdentity.CreateAsManager is auto-granted to Blueprint apps — no custom app permission required.
                     ctx.Logger.LogInformation("Creating agent identity...");
-                    var agentId = await ctx.GraphApiService.CreateAgentIdentityDelegatedAsync(
+                    if (string.IsNullOrWhiteSpace(ctx.Config.AgentBlueprintClientSecret))
+                    {
+                        ctx.Results.AgentIdentityFailed = true;
+                        ctx.Logger.LogError("Blueprint client secret is not available. Re-run 'a365 setup blueprint' to create it.");
+                        return;
+                    }
+
+                    var plainSecret = SecretProtectionHelper.UnprotectSecret(
+                        ctx.Config.AgentBlueprintClientSecret,
+                        ctx.Config.AgentBlueprintClientSecretProtected,
+                        ctx.Logger);
+                    var agentId = await ctx.GraphApiService.CreateAgentIdentityAsync(
                         ctx.Config.TenantId!,
                         ctx.Config.AgentBlueprintId!,
+                        plainSecret,
                         agentIdentityDisplayName,
                         ctx.CancellationToken);
 
@@ -451,8 +463,8 @@ private static async Task ExecuteAgentIdentityAndRegistrationAsync(
                     else if (!ctx.Results.AgentIdentityFailed)
                     {
                         ctx.Results.AgentIdentityFailed = true;
-                        ctx.Results.Warnings.Add("Agent identity creation failed. Ensure you have the Agent ID Developer or Agent ID Administrator role in this tenant.");
-                        ctx.Logger.LogWarning("Agent identity creation failed. Ensure you have the Agent ID Developer or Agent ID Administrator role in this tenant.");
+                        ctx.Results.Warnings.Add("Agent identity creation failed. Ensure blueprint setup completed and the client secret is available.");
+                        ctx.Logger.LogWarning("Agent identity creation failed. Ensure blueprint setup completed and the client secret is available.");
                     }
                 }
             }
@@ -628,18 +640,14 @@ internal static async Task GrantAgentIdentityPermissionsAsync(
             return;
         }
 
-        var agentIdentitySpObjectId = await ctx.GraphApiService.EnsureServicePrincipalForAppIdAsync(
-            ctx.Config.TenantId!,
-            ctx.Config.AgenticAppId!,
-            ctx.CancellationToken,
-            Constants.AuthenticationConstants.RequiredPermissionGrantScopes);
+        // AgenticAppId is the SP object ID returned by CreateAgentIdentityAsync and stored in config.
+        var agentIdentitySpObjectId = ctx.Config.AgenticAppId;
 
         if (string.IsNullOrWhiteSpace(agentIdentitySpObjectId))
         {
             ctx.Logger.LogWarning(
-                "Could not resolve service principal for agent identity ({AgentId}). " +
-                "Permissions must be granted manually in the Entra portal.",
-                ctx.Config.AgenticAppId);
+                "Agent identity ID not found in config. " +
+                "Permissions must be granted manually in the Entra portal.");
             return;
         }