.NET: fix InMemoryChatHistoryProvider persisting when service stores history

dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgent.cs

@@ -478,7 +478,7 @@ internal async Task NotifyProvidersOfNewMessagesAsync(
         ChatOptions? chatOptions,
         CancellationToken cancellationToken)
     {
-        ChatHistoryProvider? chatHistoryProvider = this.ResolveChatHistoryProvider(chatOptions);
+        ChatHistoryProvider? chatHistoryProvider = this.ResolveChatHistoryProvider(session, chatOptions);
 
         if (chatHistoryProvider is not null)
         {
@@ -510,7 +510,7 @@ internal async Task NotifyProvidersOfFailureAsync(
         ChatOptions? chatOptions,
         CancellationToken cancellationToken)
     {
-        ChatHistoryProvider? chatHistoryProvider = this.ResolveChatHistoryProvider(chatOptions);
+        ChatHistoryProvider? chatHistoryProvider = this.ResolveChatHistoryProvider(session, chatOptions);
 
         if (chatHistoryProvider is not null)
         {
@@ -980,22 +980,33 @@ private void WarnOnMissingPerServiceCallChatHistoryPersistingChatClient()
         }
     }
 
-    private ChatHistoryProvider? ResolveChatHistoryProvider(ChatOptions? chatOptions)
+    private ChatHistoryProvider? ResolveChatHistoryProvider(ChatClientAgentSession session, ChatOptions? chatOptions)
     {
-        ChatHistoryProvider? provider =
-            chatOptions?.ConversationId is null || IsAGUIProviderName(this._agentMetadata.ProviderName)
-            ? this.ChatHistoryProvider
-            : null;
+        // A service that manages chat history server-side disengages the chat history provider so that history
+        // is not stored in two places. The service is considered to store history when a conversation id is
+        // present either on the options (explicitly supplied by the caller) or on the session (returned by the
+        // service on a previous or the current run).
+        //
+        // The per-service-call persistence check must remain: PerServiceCallChatHistoryPersistingChatClient
+        // calls back into LoadChatHistoryAsync/NotifyProviders (which reach here) precisely when per-service-call
+        // persistence is active, and in its simulated path it stamps a sentinel onto session.ConversationId.
+        // Without this check that sentinel would be mistaken for service-stored history and wrongly disengage
+        // the provider the decorator depends on.
+        bool serviceStoresHistory =
+            !this.RequiresPerServiceCallChatHistoryPersistence
+            && !IsAGUIProviderName(this._agentMetadata.ProviderName)
+            && (!string.IsNullOrWhiteSpace(chatOptions?.ConversationId)
+                || !string.IsNullOrWhiteSpace(session.ConversationId));
+
+        ChatHistoryProvider? provider = serviceStoresHistory ? null : this.ChatHistoryProvider;
 
         // If someone provided an override ChatHistoryProvider via AdditionalProperties, we should use that instead.
         if (chatOptions?.AdditionalProperties?.TryGetValue(out ChatHistoryProvider? overrideProvider) is true)
         {
-            if (!IsAGUIProviderName(this._agentMetadata.ProviderName) &&
-                this._agentOptions?.ThrowOnChatHistoryProviderConflict is true &&
-                string.IsNullOrWhiteSpace(chatOptions?.ConversationId) is false)
+            if (this._agentOptions?.ThrowOnChatHistoryProviderConflict is true && serviceStoresHistory)
             {
                 throw new InvalidOperationException(
-                    $"Only {nameof(ChatClientAgentSession.ConversationId)} or {nameof(this.ChatHistoryProvider)} may be used, but not both. The current {nameof(ChatClientAgentSession)} has a {nameof(ChatClientAgentSession.ConversationId)} indicating server-side chat history management, but an override {nameof(this.ChatHistoryProvider)} was provided via {nameof(AgentRunOptions.AdditionalProperties)}.");
+                    $"Only {nameof(ChatClientAgentSession.ConversationId)} or {nameof(this.ChatHistoryProvider)} may be used, but not both. A {nameof(ChatClientAgentSession.ConversationId)} indicating server-side chat history management is present (on the {nameof(ChatClientAgentSession)} or the {nameof(this.ChatOptions)}), but an override {nameof(this.ChatHistoryProvider)} was provided via {nameof(AgentRunOptions.AdditionalProperties)}.");
             }
 
             // Validate that the override provider's StateKeys do not clash with any AIContextProvider's StateKeys.
@@ -1030,7 +1041,7 @@ internal async Task<IEnumerable<ChatMessage>> LoadChatHistoryAsync(
         ChatOptions? chatOptions,
         CancellationToken cancellationToken)
     {
-        var chatHistoryProvider = this.ResolveChatHistoryProvider(chatOptions);
+        var chatHistoryProvider = this.ResolveChatHistoryProvider(session, chatOptions);
         if (chatHistoryProvider is null)
         {
             return messages;

dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgent_ChatHistoryManagementTests.cs

@@ -411,6 +411,150 @@ public async Task RunAsync_DoesNotThrow_WhenNoChatHistoryProviderInOptionsAndCon
         Assert.Equal("ConvId", session!.ConversationId);
     }
 
+    /// <summary>
+    /// Regression test for https://github.com/microsoft/agent-framework/issues/6120.
+    /// When the service manages chat history server-side (returns a conversation id), the framework's
+    /// default in-memory chat history provider must not persist the messages, even on the first turn.
+    /// </summary>
+    [Fact]
+    public async Task RunAsync_DoesNotUseDefaultInMemoryChatHistoryProvider_WhenConversationIdReturnedAsync()
+    {
+        // Arrange
+        Mock<IChatClient> mockService = new();
+        mockService.Setup(
+            s => s.GetResponseAsync(
+                It.IsAny<IEnumerable<ChatMessage>>(),
+                It.IsAny<ChatOptions>(),
+                It.IsAny<CancellationToken>())).ReturnsAsync(new ChatResponse([new(ChatRole.Assistant, "response")]) { ConversationId = "ConvId" });
+        ChatClientAgent agent = new(mockService.Object, options: new()
+        {
+            ChatOptions = new() { Instructions = "test instructions" },
+        });
+
+        // Act
+        ChatClientAgentSession? session = await agent.CreateSessionAsync() as ChatClientAgentSession;
+        await agent.RunAsync([new(ChatRole.User, "test")], session);
+
+        // Assert
+        Assert.Equal("ConvId", session!.ConversationId);
+        var inMemoryProvider = Assert.IsType<InMemoryChatHistoryProvider>(agent.ChatHistoryProvider);
+        Assert.Empty(inMemoryProvider.GetMessages(session));
+    }
+
+    /// <summary>
+    /// Regression test for https://github.com/microsoft/agent-framework/issues/6120.
+    /// The streaming path must also refrain from populating the default in-memory chat history provider
+    /// when the service returns a conversation id.
+    /// </summary>
+    [Fact]
+    public async Task RunStreamingAsync_DoesNotUseDefaultInMemoryChatHistoryProvider_WhenConversationIdReturnedAsync()
+    {
+        // Arrange
+        ChatResponseUpdate[] returnUpdates =
+            [
+                new ChatResponseUpdate(role: ChatRole.Assistant, content: "response") { ConversationId = "ConvId" },
+            ];
+        Mock<IChatClient> mockService = new();
+        mockService.Setup(
+            s => s.GetStreamingResponseAsync(
+                It.IsAny<IEnumerable<ChatMessage>>(),
+                It.IsAny<ChatOptions>(),
+                It.IsAny<CancellationToken>())).Returns(returnUpdates.ToAsyncEnumerable());
+        ChatClientAgent agent = new(mockService.Object, options: new()
+        {
+            ChatOptions = new() { Instructions = "test instructions" },
+        });
+
+        // Act
+        ChatClientAgentSession? session = await agent.CreateSessionAsync() as ChatClientAgentSession;
+        await foreach (var _ in agent.RunStreamingAsync([new(ChatRole.User, "test")], session))
+        {
+        }
+
+        // Assert
+        Assert.Equal("ConvId", session!.ConversationId);
+        var inMemoryProvider = Assert.IsType<InMemoryChatHistoryProvider>(agent.ChatHistoryProvider);
+        Assert.Empty(inMemoryProvider.GetMessages(session));
+    }
+
+    /// <summary>
+    /// Regression test for https://github.com/microsoft/agent-framework/issues/6120.
+    /// Across multiple turns backed by service-stored history, the default in-memory chat history provider
+    /// is never populated and prior turns are not replayed to the service (the service owns the history).
+    /// </summary>
+    [Fact]
+    public async Task RunAsync_MultiTurnServiceStoredHistory_DoesNotPopulateDefaultInMemoryProviderAsync()
+    {
+        // Arrange
+        var capturedInputs = new List<List<ChatMessage>>();
+        Mock<IChatClient> mockService = new();
+        mockService.Setup(
+            s => s.GetResponseAsync(
+                It.IsAny<IEnumerable<ChatMessage>>(),
+                It.IsAny<ChatOptions>(),
+                It.IsAny<CancellationToken>()))
+            .Returns<IEnumerable<ChatMessage>, ChatOptions?, CancellationToken>((msgs, _, _) =>
+            {
+                capturedInputs.Add(msgs.ToList());
+                return Task.FromResult(new ChatResponse([new(ChatRole.Assistant, "response")]) { ConversationId = "ConvId" });
+            });
+        ChatClientAgent agent = new(mockService.Object, options: new()
+        {
+            ChatOptions = new() { Instructions = "test instructions" },
+        });
+
+        // Act
+        ChatClientAgentSession? session = await agent.CreateSessionAsync() as ChatClientAgentSession;
+        await agent.RunAsync([new(ChatRole.User, "first")], session);
+        await agent.RunAsync([new(ChatRole.User, "second")], session);
+
+        // Assert
+        Assert.Equal("ConvId", session!.ConversationId);
+        var inMemoryProvider = Assert.IsType<InMemoryChatHistoryProvider>(agent.ChatHistoryProvider);
+        Assert.Empty(inMemoryProvider.GetMessages(session));
+
+        // The second turn should only send the new user message, since the service owns the history.
+        Assert.Equal(2, capturedInputs.Count);
+        Assert.Single(capturedInputs[1]);
+        Assert.Equal("second", capturedInputs[1][0].Text);
+    }
+
+    /// <summary>
+    /// When the service manages chat history server-side (returns a conversation id), an explicitly-configured
+    /// chat history provider is disengaged just like the default provider, even when all conflict handling is
+    /// disabled. This pins the uniform "service storage disengages any provider" semantics.
+    /// </summary>
+    [Fact]
+    public async Task RunAsync_ExplicitChatHistoryProvider_Disengaged_WhenConflictHandlingDisabledAndConversationIdReturnedAsync()
+    {
+        // Arrange
+        Mock<IChatClient> mockService = new();
+        mockService.Setup(
+            s => s.GetResponseAsync(
+                It.IsAny<IEnumerable<ChatMessage>>(),
+                It.IsAny<ChatOptions>(),
+                It.IsAny<CancellationToken>())).ReturnsAsync(new ChatResponse([new(ChatRole.Assistant, "response")]) { ConversationId = "ConvId" });
+        var chatHistoryProvider = new InMemoryChatHistoryProvider();
+        ChatClientAgent agent = new(mockService.Object, options: new()
+        {
+            ChatOptions = new() { Instructions = "test instructions" },
+            ChatHistoryProvider = chatHistoryProvider,
+            ThrowOnChatHistoryProviderConflict = false,
+            ClearOnChatHistoryProviderConflict = false,
+            WarnOnChatHistoryProviderConflict = false,
+        });
+
+        // Act
+        ChatClientAgentSession? session = await agent.CreateSessionAsync() as ChatClientAgentSession;
+        await agent.RunAsync([new(ChatRole.User, "test")], session);
+
+        // Assert — the provider reference is retained (conflict handling disabled), but it is not persisted to
+        // because the service stores history.
+        Assert.Equal("ConvId", session!.ConversationId);
+        Assert.Same(chatHistoryProvider, agent.ChatHistoryProvider);
+        Assert.Empty(chatHistoryProvider.GetMessages(session));
+    }
+
     #endregion
 
     #region ChatHistoryProvider Override Tests