Improve blueprint app creation, secret flow

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

@@ -350,6 +350,49 @@ Create a **blocking** architectural finding if:
 - ✅ Be BALANCED: Praise good work alongside constructive criticism
 - ✅ Be ACCURATE: Only report real issues you can verify in the diff
 
+### Verification Rules (MANDATORY — prevent false positives)
+
+#### Rule 1: Mismatch Claims Require Quoted Evidence from Both Sides
+Before reporting ANY claim of the form "X doesn't match Y", "property name mismatch",
+"test uses different value than production code", or similar:
+1. Quote the **exact line from the diff** for side A (e.g. production code)
+2. Quote the **exact line from the diff** for side B (e.g. test code)
+3. Only then state whether they match or not
+
+If you cannot quote both sides verbatim from the diff, do NOT make the claim.
+
+#### Rule 2: Replacement Suggestions Must Acknowledge Behavioral Differences
+When suggesting "replace X with Y", always state explicitly whether X and Y are
+behaviorally equivalent. If they are NOT equivalent, describe the difference.
+
+Example of what NOT to do:
+  "Replace Console.WriteLine() with logger.LogInformation("")"
+  ← Wrong: these are not equivalent (logging pipeline vs. direct stdout)
+
+Example of correct form:
+  "Replace Console.WriteLine() with logger.LogInformation("") for consistency.
+   Note: these differ — Console.WriteLine always writes to stdout; LogInformation
+   is filtered by log level and can be suppressed or redirected by the logging provider."
+
+#### Rule 3: Code Suggestions Must Use Idiomatic .NET Patterns
+When suggesting a refactor to replace weak-typed constructs (e.g. string-keyed
+dictionaries, magic strings, parallel arrays), prefer the most idiomatic C# solution:
+- A small `record` or `sealed class` over two separate typed lists
+- Constants as a minimal alternative when structure change is not warranted
+- Never suggest two parallel variables/lists when a single typed container is cleaner
+
+Example:
+  ❌ Weak suggestion: "Use two typed lists: var orphanedUsers = ...; var orphanedSps = ...;"
+  ✅ Better suggestion: "Use a typed record: private sealed record OrphanedResources(...)"
+
+#### Rule 4: Blocking/High Severity Requires Verifiable Concrete Evidence
+Before marking an issue as `blocking` or `high`:
+1. You must be able to point to a specific line in the diff that demonstrates the problem
+2. For logic bugs: trace the execution path in the code to confirm the bug occurs
+3. For test failures: quote both the assertion AND the value it will actually receive
+4. If any step requires assumption or inference, lower severity to `medium` or add
+   a qualifier like "if X is true, then..." to the description
+
 ### Context Awareness
 
 Differentiate between:

src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommand.cs

@@ -235,18 +235,21 @@ public static Command CreateCommand(
                 logger.LogInformation("    - Replace 'color.png' and 'outline.png' with your custom branding");
                 logger.LogInformation("");
 
-                // Ask if user wants to open the file now
-                Console.Write("Open manifest in your default editor now? (Y/n): ");
-                var openResponse = Console.ReadLine()?.Trim().ToLowerInvariant();
-
-                if (openResponse != "n" && openResponse != "no")
+                // Ask if user wants to open the file now (skip when stdin is not a terminal)
+                if (!Console.IsInputRedirected)
                 {
-                    FileHelper.TryOpenFileInDefaultEditor(manifestPath, logger);
+                    Console.Write("Open manifest in your default editor now? (Y/n): ");
+                    var openResponse = Console.ReadLine()?.Trim().ToLowerInvariant();
+
+                    if (openResponse != "n" && openResponse != "no")
+                    {
+                        FileHelper.TryOpenFileInDefaultEditor(manifestPath, logger);
+                    }
+
+                    Console.Write("Press Enter when you have finished editing the manifest to continue with publish: ");
+                    Console.Out.Flush();
+                    Console.ReadLine();
                 }
-
-                Console.Write("Press Enter when you have finished editing the manifest to continue with publish: ");
-                Console.Out.Flush();
-                Console.ReadLine();
 
                 logger.LogInformation("Continuing with publish process...");
                 logger.LogInformation("");

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

@@ -832,7 +832,7 @@ public static async Task<bool> EnsureDelegatedConsentWithRetriesAsync(
                 };
             }
 
-            var graphToken = await GetTokenFromGraphClient(logger, graphClient, tenantId, setupConfig.ClientAppId);
+            var graphToken = await AcquireMsalGraphTokenAsync(tenantId, setupConfig.ClientAppId, logger, ct);
             if (string.IsNullOrEmpty(graphToken))
             {
                 logger.LogError("Failed to extract access token from Graph client");
@@ -862,15 +862,15 @@ public static async Task<bool> EnsureDelegatedConsentWithRetriesAsync(
             {
                 var errorContent = await appResponse.Content.ReadAsStringAsync(ct);
 
-                // If sponsors/owners fields cause error (Bad Request 400), retry without them
+                // If sponsors/owners fields cause error (Bad Request 400), retry selectively.
+                // First drop only sponsors — this preserves ownership if sponsors was the sole cause.
+                // Only drop owners as a last resort, since losing ownership breaks addPassword for non-admins.
                 if (appResponse.StatusCode == System.Net.HttpStatusCode.BadRequest &&
                     !string.IsNullOrEmpty(sponsorUserId))
                 {
-                    logger.LogWarning("Agent Blueprint creation with sponsors and owners failed (Bad Request). Retrying without sponsors/owners...");
-
-                    // Remove sponsors and owners fields and retry
+                    logger.LogWarning("Blueprint creation failed (Bad Request). Error: {Error}. Retrying without sponsors field...", errorContent);
                     appManifest.Remove("sponsors@odata.bind");
-                    appManifest.Remove("owners@odata.bind");
+                    appResponse.Dispose();
 
                     appResponse = await httpClient.PostAsync(
                         createAppUrl,
@@ -880,18 +880,46 @@ public static async Task<bool> EnsureDelegatedConsentWithRetriesAsync(
                     if (!appResponse.IsSuccessStatusCode)
                     {
                         errorContent = await appResponse.Content.ReadAsStringAsync(ct);
-                        logger.LogError("Failed to create application (fallback): {Status} - {Error}", appResponse.StatusCode, errorContent);
-                        return (false, null, null, null, alreadyExisted: false, graphPermissionsConfigured: false, graphInheritablePermissionsFailed: false, graphInheritablePermissionsError: null);
+
+                        if (appResponse.StatusCode == System.Net.HttpStatusCode.BadRequest)
+                        {
+                            logger.LogWarning("Blueprint creation without sponsors also failed (Bad Request). Error: {Error}. Retrying without owners field...", errorContent);
+                            appManifest.Remove("owners@odata.bind");
+                            appResponse.Dispose();
+
+                            appResponse = await httpClient.PostAsync(
+                                createAppUrl,
+                                new StringContent(appManifest.ToJsonString(), System.Text.Encoding.UTF8, "application/json"),
+                                ct);
+
+                            if (!appResponse.IsSuccessStatusCode)
+                            {
+                                errorContent = await appResponse.Content.ReadAsStringAsync(ct);
+                                logger.LogError("Failed to create application (all fallbacks exhausted): {Status} - {Error}", appResponse.StatusCode, errorContent);
+                                appResponse.Dispose();
+                                return (false, null, null, null, alreadyExisted: false, graphPermissionsConfigured: false, graphInheritablePermissionsFailed: false, graphInheritablePermissionsError: null);
+                            }
+
+                            logger.LogWarning("Agent Blueprint created without owner assignment. Client secret creation will fail unless the custom client app has Application.ReadWrite.All permission or you have Application Administrator role in your Entra tenant.");
+                        }
+                        else
+                        {
+                            logger.LogError("Failed to create application (fallback): {Status} - {Error}", appResponse.StatusCode, errorContent);
+                            appResponse.Dispose();
+                            return (false, null, null, null, alreadyExisted: false, graphPermissionsConfigured: false, graphInheritablePermissionsFailed: false, graphInheritablePermissionsError: null);
+                        }
                     }
                 }
                 else
                 {
                     logger.LogError("Failed to create application: {Status} - {Error}", appResponse.StatusCode, errorContent);
+                    appResponse.Dispose();
                     return (false, null, null, null, alreadyExisted: false, graphPermissionsConfigured: false, graphInheritablePermissionsFailed: false, graphInheritablePermissionsError: null);
                 }
             }
 
             var appJson = await appResponse.Content.ReadAsStringAsync(ct);
+            appResponse.Dispose();
             var app = JsonNode.Parse(appJson)!.AsObject();
             var appId = app["appId"]!.GetValue<string>();
             var objectId = app["id"]!.GetValue<string>();
@@ -1409,28 +1437,29 @@ await SetupHelpers.EnsureResourcePermissionsAsync(
     }
 
     /// <summary>
-    /// Extracts the access token from a GraphServiceClient for use in direct HTTP calls.
-    /// This uses MsalBrowserCredential, which performs platform-appropriate interactive authentication (WAM on Windows, browser-based flow on other platforms).
+    /// Acquires a Microsoft Graph access token using MSAL interactive authentication
+    /// (WAM on Windows, browser-based flow on other platforms).
+    /// The token carries the delegated permissions of the custom client app, including
+    /// Application.ReadWrite.All, which is required for operations such as addPassword.
     /// </summary>
-    private static async Task<string?> GetTokenFromGraphClient(ILogger logger, GraphServiceClient graphClient, string tenantId, string clientAppId)
+    private static async Task<string?> AcquireMsalGraphTokenAsync(string tenantId, string clientAppId, ILogger logger, CancellationToken ct = default)
     {
         try
         {
-            // Use MsalBrowserCredential which handles WAM on Windows and browser on other platforms
             var credential = new MsalBrowserCredential(
                 clientAppId,
                 tenantId,
                 redirectUri: null,  // Let MsalBrowserCredential use WAM on Windows
                 logger);
 
             var tokenRequestContext = new TokenRequestContext(new[] { "https://graph.microsoft.com/.default" });
-            var token = await credential.GetTokenAsync(tokenRequestContext, CancellationToken.None);
+            var token = await credential.GetTokenAsync(tokenRequestContext, ct);
 
             return token.Token;
         }
         catch (Exception ex)
         {
-            logger.LogError(ex, "Failed to get access token");
+            logger.LogError(ex, "Failed to acquire MSAL Graph access token");
             return null;
         }
     }
@@ -1506,12 +1535,17 @@ public static async Task CreateBlueprintClientSecretAsync(
         {
             logger.LogInformation("Creating client secret for Agent Blueprint using Graph API...");
 
-            var graphToken = await graphService.GetGraphAccessTokenAsync(
-                setupConfig.TenantId ?? string.Empty, ct);
+            // Use the MSAL token (carries Application.ReadWrite.All from the custom client app).
+            // This works for any user with a properly configured custom client app, regardless of
+            // whether they are an owner of the blueprint app registration.
+            var graphToken = await AcquireMsalGraphTokenAsync(
+                setupConfig.TenantId ?? string.Empty,
+                setupConfig.ClientAppId ?? string.Empty,
+                logger, ct);
 
             if (string.IsNullOrWhiteSpace(graphToken))
             {
-                logger.LogError("Failed to acquire Graph API access token");
+                logger.LogError("Failed to acquire MSAL Graph access token for client secret creation");
                 throw new InvalidOperationException("Cannot create client secret without Graph API token");
             }
 
@@ -1572,13 +1606,21 @@ public static async Task CreateBlueprintClientSecretAsync(
         }
         catch (Exception ex)
         {
-            logger.LogWarning(ex, "Failed to create client secret: {Message}", ex.Message);
-            logger.LogInformation("You can create a client secret manually:");
-            logger.LogInformation("  1. Go to Azure Portal > App Registrations");
-            logger.LogInformation("  2. Find your Agent Blueprint: {AppId}", blueprintAppId);
-            logger.LogInformation("  3. Navigate to Certificates & secrets > Client secrets");
-            logger.LogInformation("  4. Click 'New client secret' and save the value");
-            logger.LogInformation("  5. Add it to a365.generated.config.json as 'agentBlueprintClientSecret'");
+            logger.LogWarning(ex, "Failed to create client secret automatically: {Message}", ex.Message);
+            logger.LogWarning("To create the secret manually you need one of the following on the blueprint app registration:");
+            logger.LogWarning("  - Owner of the app registration");
+            logger.LogWarning("  - Application Administrator, Cloud Application Administrator, or Global Administrator role in your Entra tenant");
+            logger.LogWarning("See: https://learn.microsoft.com/en-us/entra/identity/role-based-access-control/permissions-reference#application-administrator");
+            logger.LogInformation("Manual steps to create and add the secret:");
+            logger.LogInformation("  1. Go to Microsoft Entra admin center (https://entra.microsoft.com)");
+            logger.LogInformation("  2. Navigate to App registrations > All applications");
+            logger.LogInformation("  3. Find your blueprint app by ID: {AppId}", blueprintAppId);
+            logger.LogInformation("  4. Open Certificates & secrets > Client secrets > New client secret");
+            logger.LogInformation("  5. Copy the Value (not the Secret ID) - it is only shown once");
+            logger.LogInformation("  6. Add both fields to a365.generated.config.json:");
+            logger.LogInformation("       \"agentBlueprintClientSecret\": \"<your secret value>\"");
+            logger.LogInformation("       \"agentBlueprintClientSecretProtected\": false");
+            logger.LogInformation("  7. Re-run: a365 setup all");
         }
     }