diff --git a/.claude/agents/pr-code-reviewer.md b/.claude/agents/pr-code-reviewer.md index 4b0e13c..3fa2764 100644 --- a/.claude/agents/pr-code-reviewer.md +++ b/.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: diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommand.cs index dd64de8..f3e422d 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommand.cs +++ b/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(""); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs index 1fcbb5e..ba68eb3 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs @@ -832,7 +832,7 @@ public static async Task 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 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 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(); var objectId = app["id"]!.GetValue(); @@ -1409,14 +1437,15 @@ await SetupHelpers.EnsureResourcePermissionsAsync( } /// - /// 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. /// - private static async Task GetTokenFromGraphClient(ILogger logger, GraphServiceClient graphClient, string tenantId, string clientAppId) + private static async Task 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, @@ -1424,13 +1453,13 @@ await SetupHelpers.EnsureResourcePermissionsAsync( 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\": \"\""); + logger.LogInformation(" \"agentBlueprintClientSecretProtected\": false"); + logger.LogInformation(" 7. Re-run: a365 setup all"); } } diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BlueprintSubcommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BlueprintSubcommandTests.cs index b9b903a..ba27f21 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BlueprintSubcommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BlueprintSubcommandTests.cs @@ -1633,6 +1633,143 @@ public async Task SetHandler_WithWhitespaceClientAppId_ShouldNotConfigureGraphAp #endregion + #region Issue-279 Regression Tests — Client Secret Creation + + // NOTE: Retry logic tests (sponsors-only fallback, owners fallback, all-fallbacks-exhausted, + // non-400 on retry 1) require HTTP call mocking. They are covered by integration tests. + // The tests below cover the observable surface: catch block logging and MSAL token path. + + [Fact] + public async Task CreateBlueprintClientSecretAsync_WhenTokenAcquisitionFails_ShouldLogPermissionsGuidance() + { + // Arrange — empty TenantId/ClientAppId causes AcquireMsalGraphTokenAsync to return null, + // which throws InvalidOperationException inside the try block, triggering the catch block. + var setupConfig = new Agent365Config + { + TenantId = string.Empty, + ClientAppId = string.Empty, + }; + + _mockConfigService.SaveStateAsync(Arg.Any(), Arg.Any()) + .Returns(Task.CompletedTask); + + // Act — should not throw; the catch block handles it + await BlueprintSubcommand.CreateBlueprintClientSecretAsync( + blueprintObjectId: "00000000-0000-0000-0000-000000000001", + blueprintAppId: "00000000-0000-0000-0000-000000000002", + graphService: _mockGraphApiService, + setupConfig: setupConfig, + configService: _mockConfigService, + logger: _mockLogger); + + // Assert — all required permission guidance must be logged + _mockLogger.Received().Log( + LogLevel.Warning, + Arg.Any(), + Arg.Is(o => o.ToString()!.Contains("Application Administrator")), + Arg.Any(), + Arg.Any>()); + + _mockLogger.Received().Log( + LogLevel.Warning, + Arg.Any(), + Arg.Is(o => o.ToString()!.Contains("Cloud Application Administrator")), + Arg.Any(), + Arg.Any>()); + } + + [Fact] + public async Task CreateBlueprintClientSecretAsync_WhenTokenAcquisitionFails_ShouldLogConfigFieldGuidance() + { + // Arrange + var setupConfig = new Agent365Config + { + TenantId = string.Empty, + ClientAppId = string.Empty, + }; + + _mockConfigService.SaveStateAsync(Arg.Any(), Arg.Any()) + .Returns(Task.CompletedTask); + + // Act + await BlueprintSubcommand.CreateBlueprintClientSecretAsync( + blueprintObjectId: "00000000-0000-0000-0000-000000000001", + blueprintAppId: "00000000-0000-0000-0000-000000000002", + graphService: _mockGraphApiService, + setupConfig: setupConfig, + configService: _mockConfigService, + logger: _mockLogger); + + // Assert — agentBlueprintClientSecretProtected: false must be mentioned + _mockLogger.Received().Log( + LogLevel.Information, + Arg.Any(), + Arg.Is(o => o.ToString()!.Contains("agentBlueprintClientSecretProtected")), + Arg.Any(), + Arg.Any>()); + } + + [Fact] + public async Task CreateBlueprintClientSecretAsync_WhenTokenAcquisitionFails_ShouldLogReRunInstruction() + { + // Arrange + var setupConfig = new Agent365Config + { + TenantId = string.Empty, + ClientAppId = string.Empty, + }; + + _mockConfigService.SaveStateAsync(Arg.Any(), Arg.Any()) + .Returns(Task.CompletedTask); + + // Act + await BlueprintSubcommand.CreateBlueprintClientSecretAsync( + blueprintObjectId: "00000000-0000-0000-0000-000000000001", + blueprintAppId: "00000000-0000-0000-0000-000000000002", + graphService: _mockGraphApiService, + setupConfig: setupConfig, + configService: _mockConfigService, + logger: _mockLogger); + + // Assert — re-run instruction must be logged + _mockLogger.Received().Log( + LogLevel.Information, + Arg.Any(), + Arg.Is(o => o.ToString()!.Contains("a365 setup all")), + Arg.Any(), + Arg.Any>()); + } + + [Fact] + public async Task CreateBlueprintClientSecretAsync_ShouldNotCallAzureCliGraphToken() + { + // Regression test for Issue #279 bug #2: + // CreateBlueprintClientSecretAsync must NOT call GetGraphAccessTokenAsync (Azure CLI token). + // It must use AcquireMsalGraphTokenAsync (MSAL token) instead. + var setupConfig = new Agent365Config + { + TenantId = string.Empty, + ClientAppId = string.Empty, + }; + + _mockConfigService.SaveStateAsync(Arg.Any(), Arg.Any()) + .Returns(Task.CompletedTask); + + // Act + await BlueprintSubcommand.CreateBlueprintClientSecretAsync( + blueprintObjectId: "00000000-0000-0000-0000-000000000001", + blueprintAppId: "00000000-0000-0000-0000-000000000002", + graphService: _mockGraphApiService, + setupConfig: setupConfig, + configService: _mockConfigService, + logger: _mockLogger); + + // Assert — Azure CLI token path must NOT be taken + await _mockGraphApiService.DidNotReceiveWithAnyArgs().GetGraphAccessTokenAsync(default!, default); + } + + #endregion + #region Mutually Exclusive Options Tests [Theory] diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/MockToolingServerSubcommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/MockToolingServerSubcommandTests.cs index ca46046..00b3b6a 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/MockToolingServerSubcommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/MockToolingServerSubcommandTests.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System.Collections.Concurrent; using Microsoft.Agents.A365.DevTools.Cli.Commands.DevelopSubcommands; using Microsoft.Agents.A365.DevTools.Cli.Services; using Microsoft.Agents.A365.DevTools.MockToolingServer; @@ -25,20 +26,26 @@ public MockToolingServerSubcommandTests() _mockProcessService = Substitute.For(); // Clear any previous state - this runs before each test - _testLogger.LogCalls.Clear(); + _testLogger.Reset(); _mockProcessService.ClearReceivedCalls(); } public void Dispose() { // Cleanup after each test if needed - _testLogger.LogCalls.Clear(); + _testLogger.Reset(); } - // Test logger that captures calls for verification + // Test logger that captures calls for verification. + // Uses ConcurrentBag so background threads can Add() while the test thread enumerates. private class TestLogger : ILogger { - public List<(LogLevel Level, string Message, object[] Args)> LogCalls { get; } = new(); + private ConcurrentBag<(LogLevel Level, string Message, object[] Args)> _logCalls = new(); + + public IReadOnlyCollection<(LogLevel Level, string Message, object[] Args)> LogCalls => _logCalls; + + // Replace the bag with a fresh empty one (called between tests — no concurrent access at that point). + public void Reset() => _logCalls = new ConcurrentBag<(LogLevel Level, string Message, object[] Args)>(); public IDisposable? BeginScope(TState state) where TState : notnull => null; public bool IsEnabled(LogLevel logLevel) => true; @@ -49,7 +56,7 @@ public void Log(LogLevel logLevel, EventId eventId, TState state, Except var args = state is IReadOnlyList> kvps ? kvps.Where(kvp => kvp.Key != "{OriginalFormat}").Select(kvp => kvp.Value ?? "").ToArray() : Array.Empty(); - LogCalls.Add((logLevel, message, args)); + _logCalls.Add((logLevel, message, args)); } } @@ -255,16 +262,18 @@ public async Task HandleStartServer_WithInvalidPort_LogsError(int invalidPort) [Fact] public async Task HandleStartServer_WithNullPort_UsesDefaultPort() { - // Act - Start task but don't await (will be cancelled by timeout) - // We're testing the initial log messages before Server.Start() blocks - var cts = new CancellationTokenSource(TimeSpan.FromMilliseconds(500)); - var task = Task.Run(async () => - { - await MockToolingServerSubcommand.HandleStartServer(null, false, false, false, _testLogger, _mockProcessService); - }, cts.Token); - - // Wait briefly for initial logging to occur - await Task.Delay(100); + // Start on a dedicated OS thread (LongRunning) so Server.Start() cannot starve the thread pool + // when many tests run in parallel. + _ = Task.Factory.StartNew( + () => MockToolingServerSubcommand.HandleStartServer(null, false, false, false, _testLogger, _mockProcessService).GetAwaiter().GetResult(), + CancellationToken.None, + TaskCreationOptions.LongRunning, + TaskScheduler.Default); + + // Poll until the initial log messages appear (up to 2 s). + var deadline = DateTime.UtcNow.AddSeconds(2); + while (!_testLogger.LogCalls.Any() && DateTime.UtcNow < deadline) + await Task.Delay(10); // Assert - Should log foreground startup messages with default port Assert.NotEmpty(_testLogger.LogCalls); @@ -284,16 +293,18 @@ public async Task HandleStartServer_WithNullPort_UsesDefaultPort() [InlineData(65535)] public async Task HandleStartServer_WithValidPort_LogsStartingMessage(int validPort) { - // Act - Start task but don't await (will be cancelled by timeout) - // We're testing the initial log messages before Server.Start() blocks - var cts = new CancellationTokenSource(TimeSpan.FromMilliseconds(500)); - var task = Task.Run(async () => - { - await MockToolingServerSubcommand.HandleStartServer(validPort, false, false, false, _testLogger, _mockProcessService); - }, cts.Token); - - // Wait briefly for initial logging to occur - await Task.Delay(100); + // Start on a dedicated OS thread (LongRunning) so Server.Start() cannot starve the thread pool + // when many tests run in parallel. + _ = Task.Factory.StartNew( + () => MockToolingServerSubcommand.HandleStartServer(validPort, false, false, false, _testLogger, _mockProcessService).GetAwaiter().GetResult(), + CancellationToken.None, + TaskCreationOptions.LongRunning, + TaskScheduler.Default); + + // Poll until the initial log messages appear (up to 2 s). + var deadline = DateTime.UtcNow.AddSeconds(2); + while (!_testLogger.LogCalls.Any() && DateTime.UtcNow < deadline) + await Task.Delay(10); // Assert - Should log foreground startup messages Assert.NotEmpty(_testLogger.LogCalls); diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandTests.cs index 0a01075..b2b1420 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandTests.cs @@ -13,11 +13,18 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Commands; +/// +/// Tests must run sequentially because some tests redirect Console.In (global state). +/// +[CollectionDefinition("PublishCommandTests", DisableParallelization = true)] +public class PublishCommandTestCollection { } + /// /// Tests for PublishCommand exit code behavior. /// These tests verify that error paths return exit code 1 and normal paths return exit code 0. /// -public class PublishCommandTests +[Collection("PublishCommandTests")] +public class PublishCommandTests : IDisposable { private readonly ILogger _logger; private readonly IConfigService _configService; @@ -25,6 +32,7 @@ public class PublishCommandTests private readonly GraphApiService _graphApiService; private readonly AgentBlueprintService _blueprintService; private readonly ManifestTemplateService _manifestTemplateService; + private readonly TextReader _originalConsoleIn = Console.In; public PublishCommandTests() { @@ -48,8 +56,15 @@ public PublishCommandTests() // ManifestTemplateService needs only ILogger _manifestTemplateService = Substitute.ForPartsOf( Substitute.For>()); + + // Auto-answer any Console.ReadLine prompts so tests do not block in environments + // where stdin is not redirected (e.g. Visual Studio Test Explorer). + // Answers: "n" = don't open editor, "" = press Enter to continue. + Console.SetIn(new StringReader("n\n\n")); } + public void Dispose() => Console.SetIn(_originalConsoleIn); + [Fact] public async Task PublishCommand_WithMissingBlueprintId_ShouldReturnExitCode1() { diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Agent365ConfigServiceTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Agent365ConfigServiceTests.cs index c4c5d1c..4fa222c 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Agent365ConfigServiceTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Agent365ConfigServiceTests.cs @@ -13,6 +13,7 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Services; /// Unit tests for ConfigService class with the new Agent365Config two-file model. /// Tests LoadAsync (merge), SaveStateAsync (split), validation, and file operations. /// +[Collection("ConfigTests")] public class Agent365ConfigServiceTests : IDisposable { private readonly string _testDirectory;