From 74905af778ee3f5585450f0d05efc7450b9d5e46 Mon Sep 17 00:00:00 2001 From: westey <164392973+westey-m@users.noreply.github.com> Date: Tue, 7 Oct 2025 09:31:34 +0100 Subject: [PATCH] .NET: Fix bug where ChatClientAgent throws when providing a ChatMessageStore with a service that requries service storage (#1229) * Fix bug where ChatClientAgent throws when providing a ChatMessageStore with a service that requries service storage * Update dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgentTests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../ChatClient/ChatClientAgent.cs | 1 - .../ChatClient/ChatClientAgentTests.cs | 145 +++++++++++++++--- 2 files changed, 124 insertions(+), 22 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgent.cs b/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgent.cs index a6a3e248b4..b9900dc3dd 100644 --- a/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgent.cs +++ b/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgent.cs @@ -322,7 +322,6 @@ public sealed class ChatClientAgent : AIAgent public override AgentThread GetNewThread() => new ChatClientAgentThread { - MessageStore = this._agentOptions?.ChatMessageStoreFactory?.Invoke(new() { SerializedState = default, JsonSerializerOptions = null }), AIContextProvider = this._agentOptions?.AIContextProviderFactory?.Invoke(new() { SerializedState = default, JsonSerializerOptions = null }) }; diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgentTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgentTests.cs index b2633c80c5..5546e8e790 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgentTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgentTests.cs @@ -2,6 +2,7 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.AI; @@ -420,6 +421,66 @@ public class ChatClientAgentTests Assert.Equal("ConvId", thread.ConversationId); } + /// + /// Verify that RunAsync uses the ChatMessageStore factory when the chat client returns no conversation id. + /// + [Fact] + public async Task RunAsyncUsesChatMessageStoreWhenNoConversationIdReturnedByChatClientAsync() + { + // Arrange + Mock mockService = new(); + mockService.Setup( + s => s.GetResponseAsync( + It.IsAny>(), + It.IsAny(), + It.IsAny())).ReturnsAsync(new ChatResponse([new(ChatRole.Assistant, "response")])); + Mock> mockFactory = new(); + mockFactory.Setup(f => f(It.IsAny())).Returns(new InMemoryChatMessageStore()); + ChatClientAgent agent = new(mockService.Object, options: new() + { + Instructions = "test instructions", + ChatMessageStoreFactory = mockFactory.Object + }); + + // Act + ChatClientAgentThread? thread = agent.GetNewThread() as ChatClientAgentThread; + await agent.RunAsync([new(ChatRole.User, "test")], thread); + + // Assert + Assert.IsType(thread!.MessageStore); + mockFactory.Verify(f => f(It.IsAny()), Times.Once); + } + + /// + /// Verify that RunAsync doesn't use the ChatMessageStore factory when the chat client returns a conversation id. + /// + [Fact] + public async Task RunAsyncIgnoresChatMessageStoreWhenConversationIdReturnedByChatClientAsync() + { + // Arrange + Mock mockService = new(); + mockService.Setup( + s => s.GetResponseAsync( + It.IsAny>(), + It.IsAny(), + It.IsAny())).ReturnsAsync(new ChatResponse([new(ChatRole.Assistant, "response")]) { ConversationId = "ConvId" }); + Mock> mockFactory = new(); + mockFactory.Setup(f => f(It.IsAny())).Returns(new InMemoryChatMessageStore()); + ChatClientAgent agent = new(mockService.Object, options: new() + { + Instructions = "test instructions", + ChatMessageStoreFactory = mockFactory.Object + }); + + // Act + ChatClientAgentThread? thread = agent.GetNewThread() as ChatClientAgentThread; + await agent.RunAsync([new(ChatRole.User, "test")], thread); + + // Assert + Assert.Equal("ConvId", thread!.ConversationId); + mockFactory.Verify(f => f(It.IsAny()), Times.Never); + } + /// /// Verify that RunAsync invokes any provided AIContextProvider and uses the result. /// @@ -1721,38 +1782,80 @@ public class ChatClientAgentTests Times.Once); } - #endregion - - #region GetNewThread Tests - + /// + /// Verify that RunStreamingAsync uses the ChatMessageStore factory when the chat client returns no conversation id. + /// [Fact] - public void GetNewThreadUsesChatMessageStoreFactoryIfProvided() + public async Task RunStreamingAsyncUsesChatMessageStoreWhenNoConversationIdReturnedByChatClientAsync() { // Arrange - var mockChatClient = new Mock(); - var mockStore = new Mock(); - var factoryCalled = false; - - var agent = new ChatClientAgent(mockChatClient.Object, new ChatClientAgentOptions + Mock mockService = new(); + ChatResponseUpdate[] returnUpdates = + [ + new ChatResponseUpdate(role: ChatRole.Assistant, content: "wh"), + new ChatResponseUpdate(role: ChatRole.Assistant, content: "at?"), + ]; + mockService.Setup( + s => s.GetStreamingResponseAsync( + It.IsAny>(), + It.IsAny(), + It.IsAny())).Returns(ToAsyncEnumerableAsync(returnUpdates)); + Mock> mockFactory = new(); + mockFactory.Setup(f => f(It.IsAny())).Returns(new InMemoryChatMessageStore()); + ChatClientAgent agent = new(mockService.Object, options: new() { - Instructions = "Test instructions", - ChatMessageStoreFactory = _ => - { - factoryCalled = true; - return mockStore.Object; - } + Instructions = "test instructions", + ChatMessageStoreFactory = mockFactory.Object }); // Act - var thread = agent.GetNewThread(); + ChatClientAgentThread? thread = agent.GetNewThread() as ChatClientAgentThread; + await agent.RunStreamingAsync([new(ChatRole.User, "test")], thread).ToListAsync(); // Assert - Assert.True(factoryCalled, "ChatMessageStoreFactory was not called."); - Assert.IsType(thread); - var typedThread = (ChatClientAgentThread)thread; - Assert.Same(mockStore.Object, typedThread.MessageStore); + Assert.IsType(thread!.MessageStore); + mockFactory.Verify(f => f(It.IsAny()), Times.Once); } + /// + /// Verify that RunStreamingAsync doesn't use the ChatMessageStore factory when the chat client returns a conversation id. + /// + [Fact] + public async Task RunStreamingAsyncIgnoresChatMessageStoreWhenConversationIdReturnedByChatClientAsync() + { + // Arrange + Mock mockService = new(); + ChatResponseUpdate[] returnUpdates = + [ + new ChatResponseUpdate(role: ChatRole.Assistant, content: "wh") { ConversationId = "ConvId" }, + new ChatResponseUpdate(role: ChatRole.Assistant, content: "at?") { ConversationId = "ConvId" }, + ]; + mockService.Setup( + s => s.GetStreamingResponseAsync( + It.IsAny>(), + It.IsAny(), + It.IsAny())).Returns(ToAsyncEnumerableAsync(returnUpdates)); + Mock> mockFactory = new(); + mockFactory.Setup(f => f(It.IsAny())).Returns(new InMemoryChatMessageStore()); + ChatClientAgent agent = new(mockService.Object, options: new() + { + Instructions = "test instructions", + ChatMessageStoreFactory = mockFactory.Object + }); + + // Act + ChatClientAgentThread? thread = agent.GetNewThread() as ChatClientAgentThread; + await agent.RunStreamingAsync([new(ChatRole.User, "test")], thread).ToListAsync(); + + // Assert + Assert.Equal("ConvId", thread!.ConversationId); + mockFactory.Verify(f => f(It.IsAny()), Times.Never); + } + + #endregion + + #region GetNewThread Tests + [Fact] public void GetNewThreadUsesAIContextProviderFactoryIfProvided() {