From 8d208f3bb367ac3136df082bd7e84cccf5ab9e3f Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Wed, 22 Apr 2026 12:51:51 +0100 Subject: [PATCH] address copilot initial feedback --- .../A2AServer/HostAgentFactory.cs | 4 +- .../AgentWebChat.AgentHost/Program.cs | 1 - .../A2AEndpointRouteBuilderExtensions.cs | 4 +- .../A2AAgentHandler.cs | 3 + .../A2AServerServiceCollectionExtensions.cs | 2 +- .../AgentRunMode.cs | 9 +- .../A2AAgentTests.cs | 25 ++ .../A2AAgentHandlerTests.cs | 32 ++ ...AServerServiceCollectionExtensionsTests.cs | 275 ++++++++++++++++++ .../AgentRunModeTests.cs | 22 +- 10 files changed, 366 insertions(+), 11 deletions(-) create mode 100644 dotnet/tests/Microsoft.Agents.AI.Hosting.A2A.UnitTests/A2AServerServiceCollectionExtensionsTests.cs diff --git a/dotnet/samples/05-end-to-end/A2AClientServer/A2AServer/HostAgentFactory.cs b/dotnet/samples/05-end-to-end/A2AClientServer/A2AServer/HostAgentFactory.cs index 8260b8302c..db2412b648 100644 --- a/dotnet/samples/05-end-to-end/A2AClientServer/A2AServer/HostAgentFactory.cs +++ b/dotnet/samples/05-end-to-end/A2AClientServer/A2AServer/HostAgentFactory.cs @@ -159,14 +159,14 @@ internal static class HostAgentFactory agentInterfaces.AddRange(agentUrls.Select(url => new AgentInterface { Url = url, - ProtocolBinding = "JSONRPC", + ProtocolBinding = ProtocolBindingNames.JsonRpc, ProtocolVersion = "1.0", })); agentInterfaces.AddRange(agentUrls.Select(url => new AgentInterface { Url = url, - ProtocolBinding = "HTTP+JSON", + ProtocolBinding = ProtocolBindingNames.HttpJson, ProtocolVersion = "1.0", })); diff --git a/dotnet/samples/05-end-to-end/AgentWebChat/AgentWebChat.AgentHost/Program.cs b/dotnet/samples/05-end-to-end/AgentWebChat/AgentWebChat.AgentHost/Program.cs index ff98f8d230..d6957a9b8e 100644 --- a/dotnet/samples/05-end-to-end/AgentWebChat/AgentWebChat.AgentHost/Program.cs +++ b/dotnet/samples/05-end-to-end/AgentWebChat/AgentWebChat.AgentHost/Program.cs @@ -1,6 +1,5 @@ // Copyright (c) Microsoft. All rights reserved. -using A2A.AspNetCore; using AgentWebChat.AgentHost; using AgentWebChat.AgentHost.Custom; using AgentWebChat.AgentHost.Utilities; diff --git a/dotnet/src/Microsoft.Agents.AI.Hosting.A2A.AspNetCore/A2AEndpointRouteBuilderExtensions.cs b/dotnet/src/Microsoft.Agents.AI.Hosting.A2A.AspNetCore/A2AEndpointRouteBuilderExtensions.cs index a57ed07890..7bfd0db8df 100644 --- a/dotnet/src/Microsoft.Agents.AI.Hosting.A2A.AspNetCore/A2AEndpointRouteBuilderExtensions.cs +++ b/dotnet/src/Microsoft.Agents.AI.Hosting.A2A.AspNetCore/A2AEndpointRouteBuilderExtensions.cs @@ -63,7 +63,7 @@ public static class A2AEndpointRouteBuilderExtensions public static IEndpointConventionBuilder MapA2AHttpJson(this IEndpointRouteBuilder endpoints, string agentName, string path) { ArgumentNullException.ThrowIfNull(endpoints); - ArgumentException.ThrowIfNullOrEmpty(agentName); + ArgumentException.ThrowIfNullOrWhiteSpace(agentName); ArgumentException.ThrowIfNullOrWhiteSpace(path); var a2aServer = endpoints.ServiceProvider.GetKeyedService(agentName) @@ -125,7 +125,7 @@ public static class A2AEndpointRouteBuilderExtensions public static IEndpointConventionBuilder MapA2AJsonRpc(this IEndpointRouteBuilder endpoints, string agentName, string path) { ArgumentNullException.ThrowIfNull(endpoints); - ArgumentException.ThrowIfNullOrEmpty(agentName); + ArgumentException.ThrowIfNullOrWhiteSpace(agentName); ArgumentException.ThrowIfNullOrWhiteSpace(path); var a2aServer = endpoints.ServiceProvider.GetKeyedService(agentName) diff --git a/dotnet/src/Microsoft.Agents.AI.Hosting.A2A/A2AAgentHandler.cs b/dotnet/src/Microsoft.Agents.AI.Hosting.A2A/A2AAgentHandler.cs index fd4a6945f1..0a57269776 100644 --- a/dotnet/src/Microsoft.Agents.AI.Hosting.A2A/A2AAgentHandler.cs +++ b/dotnet/src/Microsoft.Agents.AI.Hosting.A2A/A2AAgentHandler.cs @@ -32,6 +32,9 @@ internal sealed class A2AAgentHandler : IAgentHandler AIHostAgent hostAgent, AgentRunMode runMode) { + ArgumentNullException.ThrowIfNull(hostAgent); + ArgumentNullException.ThrowIfNull(runMode); + this._hostAgent = hostAgent; this._runMode = runMode; } diff --git a/dotnet/src/Microsoft.Agents.AI.Hosting.A2A/A2AServerServiceCollectionExtensions.cs b/dotnet/src/Microsoft.Agents.AI.Hosting.A2A/A2AServerServiceCollectionExtensions.cs index 9dbb95b989..29ab28c250 100644 --- a/dotnet/src/Microsoft.Agents.AI.Hosting.A2A/A2AServerServiceCollectionExtensions.cs +++ b/dotnet/src/Microsoft.Agents.AI.Hosting.A2A/A2AServerServiceCollectionExtensions.cs @@ -86,7 +86,7 @@ public static class A2AServerServiceCollectionExtensions public static IServiceCollection AddA2AServer(this IServiceCollection services, string agentName, Action? configureOptions = null) { ArgumentNullException.ThrowIfNull(services); - ArgumentException.ThrowIfNullOrEmpty(agentName); + ArgumentException.ThrowIfNullOrWhiteSpace(agentName); A2AServerRegistrationOptions? options = null; if (configureOptions is not null) diff --git a/dotnet/src/Microsoft.Agents.AI.Hosting.A2A/AgentRunMode.cs b/dotnet/src/Microsoft.Agents.AI.Hosting.A2A/AgentRunMode.cs index 094a5156c0..3abb90afb6 100644 --- a/dotnet/src/Microsoft.Agents.AI.Hosting.A2A/AgentRunMode.cs +++ b/dotnet/src/Microsoft.Agents.AI.Hosting.A2A/AgentRunMode.cs @@ -2,6 +2,7 @@ using System; using System.Diagnostics.CodeAnalysis; +using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; using Microsoft.Shared.DiagnosticIds; @@ -84,13 +85,17 @@ public sealed class AgentRunMode : IEquatable /// public bool Equals(AgentRunMode? other) => - other is not null && string.Equals(this._value, other._value, StringComparison.OrdinalIgnoreCase); + other is not null + && string.Equals(this._value, other._value, StringComparison.OrdinalIgnoreCase) + && ReferenceEquals(this._runInBackground, other._runInBackground); /// public override bool Equals(object? obj) => this.Equals(obj as AgentRunMode); /// - public override int GetHashCode() => StringComparer.OrdinalIgnoreCase.GetHashCode(this._value); + public override int GetHashCode() => HashCode.Combine( + StringComparer.OrdinalIgnoreCase.GetHashCode(this._value), + RuntimeHelpers.GetHashCode(this._runInBackground)); /// public override string ToString() => this._value; diff --git a/dotnet/tests/Microsoft.Agents.AI.A2A.UnitTests/A2AAgentTests.cs b/dotnet/tests/Microsoft.Agents.AI.A2A.UnitTests/A2AAgentTests.cs index 320b5b030c..f8d855b5a3 100644 --- a/dotnet/tests/Microsoft.Agents.AI.A2A.UnitTests/A2AAgentTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.A2A.UnitTests/A2AAgentTests.cs @@ -760,6 +760,31 @@ public sealed class A2AAgentTests : IDisposable Assert.Equal(TaskId, a2aSession.TaskId); } + [Fact] + public async Task RunStreamingAsync_WithContinuationToken_WhenSubscribeFailsWithNonUnsupportedError_PropagatesWithoutFallbackAsync() + { + // Arrange + const string TaskId = "error-task-123"; + + this._handler.StreamingErrorCodeToReturn = A2AErrorCode.TaskNotFound; + + var options = new AgentRunOptions { ContinuationToken = new A2AContinuationToken(TaskId) }; + + // Act & Assert - the A2AException should propagate directly without fallback to GetTask + var exception = await Assert.ThrowsAsync(async () => + { + await foreach (var _ in this._agent.RunStreamingAsync([], null, options)) + { + } + }); + + Assert.Equal(A2AErrorCode.TaskNotFound, exception.ErrorCode); + + // Assert - only SubscribeToTask was called, no fallback to GetTask + Assert.Single(this._handler.CapturedJsonRpcRequests); + Assert.Equal("SubscribeToTask", this._handler.CapturedJsonRpcRequests[0].Method); + } + [Fact] public async Task RunStreamingAsync_WithContinuationToken_WhenSubscribeAndGetTaskBothFail_PropagatesExceptionAsync() { diff --git a/dotnet/tests/Microsoft.Agents.AI.Hosting.A2A.UnitTests/A2AAgentHandlerTests.cs b/dotnet/tests/Microsoft.Agents.AI.Hosting.A2A.UnitTests/A2AAgentHandlerTests.cs index 65eeeb1fa6..512e7cc964 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Hosting.A2A.UnitTests/A2AAgentHandlerTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Hosting.A2A.UnitTests/A2AAgentHandlerTests.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; using System.Runtime.CompilerServices; +using System.Text.Json; using System.Threading; using System.Threading.Tasks; using A2A; @@ -40,6 +41,37 @@ public sealed class A2AAgentHandlerTests Assert.Null(capturedOptions.AdditionalProperties); } + /// + /// Verifies that when metadata is non-empty, the options passed to RunAsync have + /// AdditionalProperties populated with the converted metadata values. + /// + [Fact] + public async Task ExecuteAsync_WhenMetadataIsNonEmpty_PassesOptionsWithAdditionalPropertiesToRunAsync() + { + // Arrange + AgentRunOptions? capturedOptions = null; + A2AAgentHandler handler = CreateHandler(CreateAgentMock(options => capturedOptions = options)); + + // Act + await InvokeExecuteAsync(handler, new RequestContext + { + TaskId = "", ContextId = "ctx", StreamingResponse = false, + Message = new Message { MessageId = "test-id", Role = Role.User, Parts = [new Part { Text = "Hello" }] }, + Metadata = new Dictionary + { + ["key1"] = JsonSerializer.SerializeToElement("value1"), + ["key2"] = JsonSerializer.SerializeToElement(42) + } + }); + + // Assert + Assert.NotNull(capturedOptions); + Assert.False(capturedOptions.AllowBackgroundResponses); + Assert.NotNull(capturedOptions.AdditionalProperties); + Assert.Equal(2, capturedOptions.AdditionalProperties.Count); + Assert.Equal("value1", capturedOptions.AdditionalProperties["key1"]?.ToString()); + } + /// /// Verifies that when the agent response has AdditionalProperties, the returned Message.Metadata contains the converted values. /// diff --git a/dotnet/tests/Microsoft.Agents.AI.Hosting.A2A.UnitTests/A2AServerServiceCollectionExtensionsTests.cs b/dotnet/tests/Microsoft.Agents.AI.Hosting.A2A.UnitTests/A2AServerServiceCollectionExtensionsTests.cs new file mode 100644 index 0000000000..b263fd14a4 --- /dev/null +++ b/dotnet/tests/Microsoft.Agents.AI.Hosting.A2A.UnitTests/A2AServerServiceCollectionExtensionsTests.cs @@ -0,0 +1,275 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; +using A2A; +using Microsoft.Extensions.AI; +using Microsoft.Extensions.DependencyInjection; +using Moq; +using Moq.Protected; + +namespace Microsoft.Agents.AI.Hosting.A2A.UnitTests; + +/// +/// Unit tests for the class. +/// +public sealed class A2AServerServiceCollectionExtensionsTests +{ + /// + /// Verifies that AddA2AServer with an agent name registers a keyed A2AServer + /// that can be resolved from the service provider. + /// + [Fact] + public async Task AddA2AServer_WithAgentName_ResolvesKeyedA2AServerAsync() + { + // Arrange + const string AgentName = "test-agent"; + var services = new ServiceCollection(); + services.AddKeyedSingleton(AgentName, (_, _) => CreateAgentMock(AgentName).Object); + + // Act + services.AddA2AServer(AgentName); + + // Assert + await using var provider = services.BuildServiceProvider(); + var server = provider.GetKeyedService(AgentName); + Assert.NotNull(server); + } + + /// + /// Verifies that AddA2AServer with an agent instance registers a keyed A2AServer + /// that can be resolved from the service provider using the agent's name. + /// + [Fact] + public async Task AddA2AServer_WithAgentInstance_ResolvesKeyedA2AServerAsync() + { + // Arrange + const string AgentName = "instance-agent"; + var agentMock = CreateAgentMock(AgentName); + var services = new ServiceCollection(); + + // Act + services.AddA2AServer(agentMock.Object); + + // Assert + await using var provider = services.BuildServiceProvider(); + var server = provider.GetKeyedService(AgentName); + Assert.NotNull(server); + } + + /// + /// Verifies that when no ITaskStore or AgentSessionStore are registered, + /// AddA2AServer falls back to in-memory defaults and resolves successfully. + /// + [Fact] + public async Task AddA2AServer_WithNoCustomStores_FallsBackToInMemoryDefaultsAsync() + { + // Arrange + const string AgentName = "default-stores-agent"; + var services = new ServiceCollection(); + services.AddKeyedSingleton(AgentName, (_, _) => CreateAgentMock(AgentName).Object); + + // Act + services.AddA2AServer(AgentName); + + // Assert - resolution succeeds without any stores registered + await using var provider = services.BuildServiceProvider(); + var server = provider.GetKeyedService(AgentName); + Assert.NotNull(server); + } + + /// + /// Verifies that when a custom ITaskStore is registered, AddA2AServer uses it + /// instead of the default InMemoryTaskStore. + /// + [Fact] + public async Task AddA2AServer_WithCustomTaskStore_ResolvesSuccessfullyAsync() + { + // Arrange + const string AgentName = "custom-taskstore-agent"; + var services = new ServiceCollection(); + services.AddKeyedSingleton(AgentName, (_, _) => CreateAgentMock(AgentName).Object); + + var mockTaskStore = new Mock(); + services.AddKeyedSingleton(AgentName, mockTaskStore.Object); + + // Act + services.AddA2AServer(AgentName); + + // Assert + await using var provider = services.BuildServiceProvider(); + var server = provider.GetKeyedService(AgentName); + Assert.NotNull(server); + } + + /// + /// Verifies that when a custom AgentSessionStore is registered, AddA2AServer uses it + /// instead of the default InMemoryAgentSessionStore. + /// + [Fact] + public async Task AddA2AServer_WithCustomAgentSessionStore_ResolvesSuccessfullyAsync() + { + // Arrange + const string AgentName = "custom-sessionstore-agent"; + var services = new ServiceCollection(); + services.AddKeyedSingleton(AgentName, (_, _) => CreateAgentMock(AgentName).Object); + + var mockSessionStore = new Mock(); + services.AddKeyedSingleton(AgentName, mockSessionStore.Object); + + // Act + services.AddA2AServer(AgentName); + + // Assert + await using var provider = services.BuildServiceProvider(); + var server = provider.GetKeyedService(AgentName); + Assert.NotNull(server); + } + + /// + /// Verifies that when a custom IAgentHandler is registered, AddA2AServer uses it + /// instead of creating a default A2AAgentHandler. + /// + [Fact] + public async Task AddA2AServer_WithCustomAgentHandler_ResolvesSuccessfullyAsync() + { + // Arrange + const string AgentName = "custom-handler-agent"; + var services = new ServiceCollection(); + services.AddKeyedSingleton(AgentName, (_, _) => CreateAgentMock(AgentName).Object); + + var mockHandler = new Mock(); + services.AddKeyedSingleton(AgentName, mockHandler.Object); + + // Act + services.AddA2AServer(AgentName); + + // Assert + await using var provider = services.BuildServiceProvider(); + var server = provider.GetKeyedService(AgentName); + Assert.NotNull(server); + } + + /// + /// Verifies that the configureOptions callback is invoked when provided. + /// + [Fact] + public async Task AddA2AServer_WithConfigureOptions_InvokesCallbackAsync() + { + // Arrange + const string AgentName = "options-agent"; + var services = new ServiceCollection(); + services.AddKeyedSingleton(AgentName, (_, _) => CreateAgentMock(AgentName).Object); + + bool callbackInvoked = false; + + // Act + services.AddA2AServer(AgentName, options => + { + callbackInvoked = true; + options.AgentRunMode = AgentRunMode.AllowBackgroundIfSupported; + }); + + // Assert - callback is invoked during resolution + await using var provider = services.BuildServiceProvider(); + var server = provider.GetKeyedService(AgentName); + Assert.NotNull(server); + Assert.True(callbackInvoked); + } + + /// + /// Verifies that AddA2AServer with a null configureOptions does not throw. + /// + [Fact] + public async Task AddA2AServer_WithNullConfigureOptions_ResolvesSuccessfullyAsync() + { + // Arrange + const string AgentName = "null-options-agent"; + var services = new ServiceCollection(); + services.AddKeyedSingleton(AgentName, (_, _) => CreateAgentMock(AgentName).Object); + + // Act + services.AddA2AServer(AgentName, configureOptions: null); + + // Assert + await using var provider = services.BuildServiceProvider(); + var server = provider.GetKeyedService(AgentName); + Assert.NotNull(server); + } + + /// + /// Verifies that AddA2AServer throws when the agent name is null. + /// + [Fact] + public void AddA2AServer_WithNullAgentName_ThrowsArgumentException() + { + // Arrange + var services = new ServiceCollection(); + + // Act & Assert + Assert.ThrowsAny(() => services.AddA2AServer(agentName: null!)); + } + + /// + /// Verifies that AddA2AServer throws when the agent name is whitespace. + /// + [Fact] + public void AddA2AServer_WithWhitespaceAgentName_ThrowsArgumentException() + { + // Arrange + var services = new ServiceCollection(); + + // Act & Assert + Assert.ThrowsAny(() => services.AddA2AServer(agentName: " ")); + } + + /// + /// Verifies that AddA2AServer throws when the services parameter is null. + /// + [Fact] + public void AddA2AServer_WithNullServices_ThrowsArgumentNullException() + { + // Arrange + IServiceCollection services = null!; + + // Act & Assert + Assert.Throws(() => services.AddA2AServer("agent")); + } + + /// + /// Verifies that AddA2AServer with an agent instance throws when the agent is null. + /// + [Fact] + public void AddA2AServer_WithNullAgent_ThrowsArgumentNullException() + { + // Arrange + var services = new ServiceCollection(); + + // Act & Assert + Assert.Throws(() => services.AddA2AServer(agent: null!)); + } + + private static Mock CreateAgentMock(string name) + { + Mock agentMock = new() { CallBase = true }; + agentMock.SetupGet(x => x.Name).Returns(name); + agentMock + .Protected() + .Setup>("CreateSessionCoreAsync", ItExpr.IsAny()) + .ReturnsAsync(new TestAgentSession()); + agentMock + .Protected() + .Setup>("RunCoreAsync", + ItExpr.IsAny>(), + ItExpr.IsAny(), + ItExpr.IsAny(), + ItExpr.IsAny()) + .ReturnsAsync(new AgentResponse([new ChatMessage(ChatRole.Assistant, "Test response")])); + + return agentMock; + } + + private sealed class TestAgentSession : AgentSession; +} diff --git a/dotnet/tests/Microsoft.Agents.AI.Hosting.A2A.UnitTests/AgentRunModeTests.cs b/dotnet/tests/Microsoft.Agents.AI.Hosting.A2A.UnitTests/AgentRunModeTests.cs index 6128e0eefc..1114e39c31 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Hosting.A2A.UnitTests/AgentRunModeTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Hosting.A2A.UnitTests/AgentRunModeTests.cs @@ -130,16 +130,32 @@ public sealed class AgentRunModeTests } /// - /// Verifies that two AllowBackgroundWhen instances with different delegates are considered equal, - /// because equality is based on the mode value ("dynamic"), not the delegate. + /// Verifies that two AllowBackgroundWhen instances with different delegates are not considered equal, + /// because equality includes delegate identity for dynamic modes. /// [Fact] - public void Equals_AllowBackgroundWhen_DifferentDelegates_AreEqual() + public void Equals_AllowBackgroundWhen_DifferentDelegates_AreNotEqual() { // Arrange var mode1 = AgentRunMode.AllowBackgroundWhen((_, _) => ValueTask.FromResult(true)); var mode2 = AgentRunMode.AllowBackgroundWhen((_, _) => ValueTask.FromResult(false)); + // Act & Assert + Assert.False(mode1.Equals(mode2)); + Assert.True(mode1 != mode2); + } + + /// + /// Verifies that two AllowBackgroundWhen instances with the same delegate are considered equal. + /// + [Fact] + public void Equals_AllowBackgroundWhen_SameDelegate_AreEqual() + { + // Arrange + Func> callback = (_, _) => ValueTask.FromResult(true); + var mode1 = AgentRunMode.AllowBackgroundWhen(callback); + var mode2 = AgentRunMode.AllowBackgroundWhen(callback); + // Act & Assert Assert.True(mode1.Equals(mode2)); Assert.True(mode1 == mode2);