From 49b52b14eab1d132c30a2a48138fa3266d251812 Mon Sep 17 00:00:00 2001 From: westey <164392973+westey-m@users.noreply.github.com> Date: Mon, 30 Mar 2026 13:28:52 +0000 Subject: [PATCH 1/6] Allow Simulating service stored ChatHistory to improve consistency --- ...22-chat-history-persistence-consistency.md | 64 ++-- .../Program.cs | 10 +- .../README.md | 13 +- .../ChatClient/ChatClientAgent.cs | 213 +++++------ .../ChatClient/ChatClientAgentLogMessages.cs | 8 +- .../ChatClient/ChatClientAgentOptions.cs | 52 +-- .../ChatClient/ChatClientBuilderExtensions.cs | 28 +- .../ChatClient/ChatClientExtensions.cs | 19 +- .../ChatHistoryPersistingChatClient.cs | 351 ------------------ .../ServiceStoredSimulatingChatClient.cs | 239 ++++++++++++ .../ChatClient/ChatClientAgentTestHelper.cs | 2 +- .../ChatClient/ChatClientAgentTests.cs | 11 +- .../ChatClientAgent_ApprovalsTests.cs | 10 +- ...tClientAgent_ChatHistoryManagementTests.cs | 7 +- ...ChatClientAgent_ChatOptionsMergingTests.cs | 12 +- ...ServiceStoredSimulatingChatClientTests.cs} | 133 +++---- 16 files changed, 472 insertions(+), 700 deletions(-) delete mode 100644 dotnet/src/Microsoft.Agents.AI/ChatClient/ChatHistoryPersistingChatClient.cs create mode 100644 dotnet/src/Microsoft.Agents.AI/ChatClient/ServiceStoredSimulatingChatClient.cs rename dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/{ChatHistoryPersistingChatClientTests.cs => ServiceStoredSimulatingChatClientTests.cs} (87%) diff --git a/docs/decisions/0022-chat-history-persistence-consistency.md b/docs/decisions/0022-chat-history-persistence-consistency.md index 2c760e6614..08a31bc88a 100644 --- a/docs/decisions/0022-chat-history-persistence-consistency.md +++ b/docs/decisions/0022-chat-history-persistence-consistency.md @@ -31,8 +31,6 @@ The persistence timing and `FunctionResultContent` trimming behaviors are interr - **Per-run persistence**: When messages are batched and persisted at the end of the full run, trailing `FunctionResultContent` trimming becomes necessary to match the service's behavior. Without trimming, the stored history contains `FunctionResultContent` that the service would never have stored. -This means the trimming feature (introduced in [PR #4792](https://github.com/microsoft/agent-framework/pull/4792)) is primarily needed as a complement to per-run persistence. The `PersistChatHistoryAtEndOfRun` setting (introduced in [PR #4762](https://github.com/microsoft/agent-framework/pull/4762)) inverts the default so that per-service-call persistence is the standard behavior, and per-run persistence is opt-in. - ## Decision Drivers - **A. Consistency**: The default behavior of `ChatHistoryProvider` should produce stored history that closely matches what the underlying AI service would store, minimizing surprise when switching between framework-managed and service-managed chat history. @@ -43,33 +41,30 @@ This means the trimming feature (introduced in [PR #4792](https://github.com/mic ## Considered Options -- Option 1: Default to per-run persistence with `FunctionResultContent` trimming (opt-in to per-service-call) -- Option 2: Default to per-service-call persistence (opt-in to per-run) +- Option 1: Per-run persistence with opt-in FRC trimming +- Option 2: Opt-in per-service-call persistence (via `SimulateServiceStoredChatHistory`) ## Pros and Cons of the Options -### Option 1: Default to per-run persistence with `FunctionResultContent` trimming - -Keep the current default behavior of persisting chat history only at the end of the full agent run. Add `FunctionResultContent` trimming as the default to improve consistency with service storage. Provide an opt-in setting for users who want per-service-call persistence. +### Option 1: Per-run persistence with opt-in FRC trimming -Settings: -- `PersistChatHistoryAtEndOfRun` = `true` +Keep the current default behavior of persisting chat history only at the end of the full agent run. Add `FunctionResultContent` trimming as an opt-in behavior to improve consistency with service storage. - Good, because runs are atomic — chat history is only updated when the full run succeeds, satisfying driver B. - Good, because the mental model is simple: one run = one history update, satisfying driver D. - Good, because trimming trailing `FunctionResultContent` improves consistency with service storage, partially satisfying driver A. -- Good, because users can opt in to per-service-call persistence for checkpointing/recovery scenarios, satisfying drivers C and E. - Bad, because the default persistence timing still differs from the service's behavior (per-run vs. per-service-call), only partially satisfying driver A. -- Bad, because if the process crashes mid-loop, all intermediate progress from the current run is lost, not satisfying driver C by default. +- Bad, because if the process crashes mid-loop, all intermediate progress from the current run is lost, not satisfying driver C. +- Bad, because this option alone does not provide a way for users to opt into per-service-call persistence, not satisfying driver E. -### Option 2: Default to per-service-call persistence +### Option 2: Opt-in per-service-call persistence (via `SimulateServiceStoredChatHistory`) -Change the default to persist chat history after each individual service call within the FIC loop, matching the AI service's behavior. Trailing `FunctionResultContent` trimming is unnecessary with this approach (it is naturally handled). Provide an opt-in setting for users who want per-run atomicity with trimming. +Introduce an optional SimulateServiceStoredChatHistory setting to persist chat history after each individual service call within the FIC loop, matching the AI service's behavior. Trailing `FunctionResultContent` trimming is unnecessary with this approach (it is naturally handled). Settings: -- `PersistChatHistoryAtEndOfRun` = `false` (default) +- `SimulateServiceStoredChatHistory` = `true` -- Good, because the stored history matches the service's behavior by default for both timing and content, fully satisfying driver A. +- Good, because the stored history matches the service's behavior when opting in for both timing and content, fully satisfying driver A. - Good, because intermediate progress is preserved if the process is interrupted, satisfying driver C. - Good, because no separate `FunctionResultContent` trimming logic is needed, reducing complexity. - Bad, because chat history may be left in an incomplete state if the run fails mid-loop (e.g., `FunctionCallContent` stored without corresponding `FunctionResultContent`), not satisfying driver B. A subsequent run cannot proceed without manually providing the missing `FunctionResultContent`. @@ -78,39 +73,36 @@ Settings: ## Decision Outcome -Chosen option: **Option 2 — Default to per-service-call persistence**, because it fully satisfies the consistency driver (A), naturally handles `FunctionResultContent` trimming without additional logic, and provides better recoverability for long-running tool-calling loops. Per-run persistence remains available via the `PersistChatHistoryAtEndOfRun` setting for users who prefer atomic run semantics. +Chosen option: **Option 2: Opt-in per-service-call persistence (via `SimulateServiceStoredChatHistory`)**. The existing per-run persistence behavior is retained as-is, requiring no changes from users. Per-service-call persistence is available as an opt-in feature via the `SimulateServiceStoredChatHistory` setting. This satisfies drivers B (atomicity) and D (simplicity) for the common case, while fully satisfying driver A (consistency) for users who opt into simulated service-stored behavior. Users who need per-service-call persistence for recoverability (driver C) can enable it explicitly. ### Configuration Matrix -The behavior depends on the combination of `UseProvidedChatClientAsIs` and `PersistChatHistoryAtEndOfRun`: +The behavior depends on the combination of `UseProvidedChatClientAsIs` and `SimulateServiceStoredChatHistory`: -| `UseProvidedChatClientAsIs` | `PersistChatHistoryAtEndOfRun` | Behavior | +| `UseProvidedChatClientAsIs` | `SimulateServiceStoredChatHistory` | Behavior | |---|---|---| -| `false` (default) | `false` (default) | **Per-service-call persistence.** A `ChatHistoryPersistingChatClient` middleware is automatically injected into the chat client pipeline between `FunctionInvokingChatClient` and the leaf `IChatClient`. Messages are persisted after each service call. | -| `true` | `false` | **User responsibility.** No middleware is injected because the user has provided a custom chat client stack. The user is responsible for ensuring correct persistence behavior (e.g., by including their own persisting middleware). | -| `false` | `true` | **Per-run persistence with marking.** A `ChatHistoryPersistingChatClient` middleware is injected, but configured to *mark* messages with metadata rather than store them immediately. At the end of the run, marked messages are stored. Trailing `FunctionResultContent` is trimmed. | -| `true` | `true` | **Per-run persistence with warning.** The system checks whether the custom chat client stack includes a `ChatHistoryPersistingChatClient`. If not, a warning is emitted (particularly relevant for workflow handoff scenarios where trimming cannot be guaranteed). If no `ChatHistoryPersistingChatClient` is preset, all messages are stored at the end of the run, otherwise marked messages are stored. | +| `false` (default) | `false` (default) | **Per-run persistence.** Messages are persisted at the end of the full agent run via the `ChatHistoryProvider`. | +| `false` | `true` | **Per-service-call persistence (simulated).** A `ServiceStoredSimulatingChatClient` middleware is automatically injected into the chat client pipeline between `FunctionInvokingChatClient` and the leaf `IChatClient`. Messages are persisted after each service call. A sentinel `ConversationId` causes FIC to treat the conversation as service-managed. | +| `true` | `false` | **Per-run persistence.** No middleware is injected because the user has provided a custom chat client stack. Messages are persisted at the end of the run. | +| `true` | `true` | **User responsibility.** The system checks whether the custom chat client stack includes a `ServiceStoredSimulatingChatClient`. If not, a warning is emitted — the user is expected to have added their own per-service-call persistence mechanism. End-of-run persistence is skipped. | ### Consequences -- Good, because the stored history matches the service's behavior by default for both timing and content, fully satisfying consistency (driver A). -- Good, because intermediate progress is preserved if the process is interrupted, satisfying recoverability (driver C). -- Good, because no separate `FunctionResultContent` trimming logic is needed in the default path, reducing complexity. -- Good, because marking persisted messages with metadata enables deduplication and aids debugging. -- Good, because warnings for custom chat client configurations without the persisting middleware help prevent silent failures in workflow handoff scenarios. -- Bad, because chat history may be left in an incomplete state if the run fails mid-loop (e.g., `FunctionCallContent` stored without corresponding `FunctionResultContent`), requiring manual recovery in rare cases. -- Bad, because the mental model is more complex for the default path: a single run may produce multiple history updates. -- Neutral, because users who prefer atomic run semantics can opt in to per-run persistence via `PersistChatHistoryAtEndOfRun = true`. +- Good, because per-run persistence is atomic by default — chat history is only updated when the full run succeeds, satisfying driver B. +- Good, because the default mental model is simple: one run = one history update, satisfying driver D. +- Good, because users who opt into `SimulateServiceStoredChatHistory` get stored history that matches the service's behavior for both timing and content, fully satisfying driver A. +- Good, because per-service-call persistence preserves intermediate progress if the process is interrupted, satisfying driver C when opted in. +- Good, because no separate `FunctionResultContent` trimming logic is needed when per-service-call persistence is active — it is naturally handled. +- Good, because conflict detection (configurable via `ThrowOnChatHistoryProviderConflict`, `WarnOnChatHistoryProviderConflict`, `ClearOnChatHistoryProviderConflict`) prevents misconfiguration when a service returns a `ConversationId` alongside a configured `ChatHistoryProvider`. +- Bad, because per-service-call persistence (when opted in) may leave chat history in an incomplete state if the run fails mid-loop (e.g., `FunctionCallContent` stored without corresponding `FunctionResultContent`), requiring manual recovery in rare cases. +- Neutral, because users who want per-service-call consistency can opt in via `SimulateServiceStoredChatHistory = true`, satisfying driver E. - Neutral, because increased write frequency from per-service-call persistence may impact performance for some storage backends; this can be mitigated with a caching decorator. ### Implementation Notes #### Conversation ID Consistency -The `ChatHistoryPersistingChatClient` middleware must also update the session's `ConversationId` consistently for both response-based and conversation-based service interactions, ensuring the session always reflects the latest service-provided identifier. - -## More Information +We should introduce a separate `ConversationIdPersistingChatClient`, middleware which allows us to +persist response `ConversationIds` during the FICC loop. This could be used with or without +`ServiceStoredSimulatingChatClient`. -- [PR #4762: Persist messages during function call loop](https://github.com/microsoft/agent-framework/pull/4762) — introduces `PersistChatHistoryAfterEachServiceCall` option and `ChatHistoryPersistingChatClient` decorator -- [PR #4792: Trim final FRC to match service storage](https://github.com/microsoft/agent-framework/pull/4792) — introduces `StoreFinalFunctionResultContent` option and `FilterFinalFunctionResultContent` logic -- [Issue #2889](https://github.com/microsoft/agent-framework/issues/2889) — original issue tracking chat history persistence during function call loops diff --git a/dotnet/samples/02-agents/Agents/Agent_Step19_InFunctionLoopCheckpointing/Program.cs b/dotnet/samples/02-agents/Agents/Agent_Step19_InFunctionLoopCheckpointing/Program.cs index 07382e6417..0c74ad86b1 100644 --- a/dotnet/samples/02-agents/Agents/Agent_Step19_InFunctionLoopCheckpointing/Program.cs +++ b/dotnet/samples/02-agents/Agents/Agent_Step19_InFunctionLoopCheckpointing/Program.cs @@ -1,15 +1,16 @@ // Copyright (c) Microsoft. All rights reserved. // This sample demonstrates how the ChatClientAgent persists chat history after each individual -// call to the AI service. +// call to the AI service, using the SimulateServiceStoredChatHistory option. // When an agent uses tools, FunctionInvokingChatClient may loop multiple times // (service call → tool execution → service call), and intermediate messages (tool calls and // results) are persisted after each service call. This allows you to inspect or recover them // even if the process is interrupted mid-loop, but may also result in chat history that is not // yet finalized (e.g., tool calls without results) being persisted, which may be undesirable in some cases. // -// To opt into end-of-run persistence instead (atomic run semantics), set -// PersistChatHistoryAtEndOfRun = true on ChatClientAgentOptions. +// To use end-of-run persistence instead (atomic run semantics), remove the +// SimulateServiceStoredChatHistory = true setting (or set it to false). End-of-run +// persistence is the default behavior. // // The sample runs two multi-turn conversations: one using non-streaming (RunAsync) and one // using streaming (RunStreamingAsync), to demonstrate correct behavior in both modes. @@ -53,7 +54,7 @@ static string GetTime([Description("The city name.")] string city) => _ => $"{city}: time data not available." }; -// Create the agent — per-service-call persistence is the default behavior. +// Create the agent — per-service-call persistence is enabled via SimulateServiceStoredChatHistory. // The in-memory ChatHistoryProvider is used by default when the service does not require service stored chat // history, so for those cases, we can inspect the chat history via session.TryGetInMemoryChatHistory(). IChatClient chatClient = string.Equals(store, "TRUE", StringComparison.OrdinalIgnoreCase) ? @@ -63,6 +64,7 @@ static string GetTime([Description("The city name.")] string city) => new ChatClientAgentOptions { Name = "WeatherAssistant", + SimulateServiceStoredChatHistory = true, ChatOptions = new() { Instructions = "You are a helpful assistant. When asked about multiple cities, call the appropriate tool for each city.", diff --git a/dotnet/samples/02-agents/Agents/Agent_Step19_InFunctionLoopCheckpointing/README.md b/dotnet/samples/02-agents/Agents/Agent_Step19_InFunctionLoopCheckpointing/README.md index d6157586f0..461f76aaf4 100644 --- a/dotnet/samples/02-agents/Agents/Agent_Step19_InFunctionLoopCheckpointing/README.md +++ b/dotnet/samples/02-agents/Agents/Agent_Step19_InFunctionLoopCheckpointing/README.md @@ -1,16 +1,19 @@ # In-Function-Loop Checkpointing -This sample demonstrates how `ChatClientAgent` persists chat history after each individual call to the AI service by default. This per-service-call persistence ensures intermediate progress is saved during the function invocation loop. +This sample demonstrates how `ChatClientAgent` can persist chat history after each individual call to the AI service using the `SimulateServiceStoredChatHistory` option. This per-service-call persistence ensures intermediate progress is saved during the function invocation loop. ## What This Sample Shows -When an agent uses tools, the `FunctionInvokingChatClient` loops multiple times (service call → tool execution → service call → …). By default, chat history is persisted after each service call via the `ChatHistoryPersistingChatClient` decorator: +When an agent uses tools, the `FunctionInvokingChatClient` loops multiple times (service call → tool execution → service call → …). By enabling `SimulateServiceStoredChatHistory = true`, chat history is persisted after each service call via the `ServiceStoredSimulatingChatClient` decorator: -- A `ChatHistoryPersistingChatClient` decorator is automatically inserted into the chat client pipeline +- A `ServiceStoredSimulatingChatClient` decorator is inserted into the chat client pipeline +- Before each service call, the decorator loads history from the `ChatHistoryProvider` and prepends it to the request - After each service call, the decorator notifies the `ChatHistoryProvider` (and any `AIContextProvider` instances) with the new messages - Only **new** messages are sent to providers on each notification — messages that were already persisted in an earlier call within the same run are deduplicated automatically -To opt into end-of-run persistence instead (atomic run semantics), set `PersistChatHistoryAtEndOfRun = true` on `ChatClientAgentOptions`. In that mode, the decorator marks messages with metadata rather than persisting them immediately, and `ChatClientAgent` persists only the marked messages at the end of the run. +By default (without `SimulateServiceStoredChatHistory`), chat history is persisted at the end of the full agent run instead. To use per-service-call persistence, set `SimulateServiceStoredChatHistory = true` on `ChatClientAgentOptions`. + +With `SimulateServiceStoredChatHistory` = true, the behavior matches that of chat history stored in the underlying AI service exactly. Per-service-call persistence is useful for: - **Crash recovery** — if the process is interrupted mid-loop, the intermediate tool calls and results are already persisted @@ -26,7 +29,7 @@ The sample asks the agent about the weather and time in three cities. The model ``` ChatClientAgent └─ FunctionInvokingChatClient (handles tool call loop) - └─ ChatHistoryPersistingChatClient (persists after each service call) + └─ ServiceStoredSimulatingChatClient (persists after each service call) └─ Leaf IChatClient (Azure OpenAI) ``` diff --git a/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgent.cs b/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgent.cs index 05caff4d83..49f4324397 100644 --- a/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgent.cs +++ b/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgent.cs @@ -139,8 +139,8 @@ public ChatClientAgent(IChatClient chatClient, ChatClientAgentOptions? options, this._logger = (loggerFactory ?? chatClient.GetService() ?? NullLoggerFactory.Instance).CreateLogger(); - // Warn if using a custom chat client stack with end-of-run persistence but no ChatHistoryPersistingChatClient. - this.WarnOnMissingPersistingClient(); + // Warn if using a custom chat client stack with simulated service stored persistence but no ServiceStoredSimulatingChatClient. + this.WarnOnMissingServiceStoredSimulatingClient(); } /// @@ -454,7 +454,7 @@ protected override ValueTask DeserializeSessionCoreAsync(JsonEleme /// Notifies the and all of successfully completed messages. /// /// - /// This method is also called by to persist messages per-service-call. + /// This method is also called by to persist messages per-service-call. /// internal async Task NotifyProvidersOfNewMessagesAsync( ChatClientAgentSession session, @@ -463,7 +463,7 @@ internal async Task NotifyProvidersOfNewMessagesAsync( ChatOptions? chatOptions, CancellationToken cancellationToken) { - ChatHistoryProvider? chatHistoryProvider = this.ResolveChatHistoryProvider(chatOptions, session); + ChatHistoryProvider? chatHistoryProvider = this.ResolveChatHistoryProvider(chatOptions); if (chatHistoryProvider is not null) { @@ -486,7 +486,7 @@ internal async Task NotifyProvidersOfNewMessagesAsync( /// Notifies the and all of a failure during a service call. /// /// - /// This method is also called by to report failures per-service-call. + /// This method is also called by to report failures per-service-call. /// internal async Task NotifyProvidersOfFailureAsync( ChatClientAgentSession session, @@ -495,7 +495,7 @@ internal async Task NotifyProvidersOfFailureAsync( ChatOptions? chatOptions, CancellationToken cancellationToken) { - ChatHistoryProvider? chatHistoryProvider = this.ResolveChatHistoryProvider(chatOptions, session); + ChatHistoryProvider? chatHistoryProvider = this.ResolveChatHistoryProvider(chatOptions); if (chatHistoryProvider is not null) { @@ -701,7 +701,7 @@ private async Task throw new InvalidOperationException("A session must be provided when continuing a background response with a continuation token."); } - if ((continuationToken is not null || chatOptions?.AllowBackgroundResponses is true) && this.PersistsChatHistoryPerServiceCall && this._logger.IsEnabled(LogLevel.Warning)) + if ((continuationToken is not null || chatOptions?.AllowBackgroundResponses is true) && this.SimulatesServiceStoredChatHistory && this._logger.IsEnabled(LogLevel.Warning)) { var warningAgentName = this.GetLoggingAgentName(); this._logger.LogAgentChatClientBackgroundResponseFallback(this.Id, warningAgentName); @@ -721,52 +721,49 @@ private async Task IEnumerable inputMessagesForChatClient = inputMessages; - // Populate the session messages only if we are not continuing an existing response as it's not allowed - if (chatOptions?.ContinuationToken is null) + // Populate the session messages only if we are not continuing an existing response as it's not allowed. + // When SimulateServiceStoredChatHistory is active, the ServiceStoredSimulatingChatClient + // owns the chat history lifecycle — it loads history before each service call. The agent + // must not load history itself, as that would result in duplicate messages. + if (chatOptions?.ContinuationToken is null && !this.SimulatesServiceStoredChatHistory) { - ChatHistoryProvider? chatHistoryProvider = this.ResolveChatHistoryProvider(chatOptions, typedSession); - // Add any existing messages from the session to the messages to be sent to the chat client. // The ChatHistoryProvider returns the merged result (history + input messages). - if (chatHistoryProvider is not null) - { - var invokingContext = new ChatHistoryProvider.InvokingContext(this, typedSession, inputMessagesForChatClient); - inputMessagesForChatClient = await chatHistoryProvider.InvokingAsync(invokingContext, cancellationToken).ConfigureAwait(false); - } + inputMessagesForChatClient = await this.LoadChatHistoryAsync(typedSession, inputMessagesForChatClient, chatOptions, cancellationToken).ConfigureAwait(false); + } - // If we have an AIContextProvider, we should get context from it, and update our - // messages and options with the additional context. - // The AIContextProvider returns the accumulated AIContext (original + new contributions). - if (this.AIContextProviders is { Count: > 0 } aiContextProviders) + // AIContextProviders should always be invoked (unless continuing an existing response) + // to contribute additional messages, tools, and instructions — even when the decorator + // handles history loading. + if (chatOptions?.ContinuationToken is null && this.AIContextProviders is { Count: > 0 } aiContextProviders) + { + var aiContext = new AIContext { - var aiContext = new AIContext - { - Instructions = chatOptions?.Instructions, - Messages = inputMessagesForChatClient, - Tools = chatOptions?.Tools - }; + Instructions = chatOptions?.Instructions, + Messages = inputMessagesForChatClient, + Tools = chatOptions?.Tools + }; - foreach (var aiContextProvider in aiContextProviders) - { - var invokingContext = new AIContextProvider.InvokingContext(this, typedSession, aiContext); - aiContext = await aiContextProvider.InvokingAsync(invokingContext, cancellationToken).ConfigureAwait(false); - } + foreach (var aiContextProvider in aiContextProviders) + { + var invokingContext = new AIContextProvider.InvokingContext(this, typedSession, aiContext); + aiContext = await aiContextProvider.InvokingAsync(invokingContext, cancellationToken).ConfigureAwait(false); + } - // Materialize the accumulated messages and tools once at the end of the provider pipeline. - inputMessagesForChatClient = aiContext.Messages ?? []; + // Materialize the accumulated messages and tools once at the end of the provider pipeline. + inputMessagesForChatClient = aiContext.Messages ?? []; - var tools = aiContext.Tools as IList ?? aiContext.Tools?.ToList(); - if (chatOptions?.Tools is { Count: > 0 } || tools is { Count: > 0 }) - { - chatOptions ??= new(); - chatOptions.Tools = tools; - } + var tools = aiContext.Tools as IList ?? aiContext.Tools?.ToList(); + if (chatOptions?.Tools is { Count: > 0 } || tools is { Count: > 0 }) + { + chatOptions ??= new(); + chatOptions.Tools = tools; + } - if (chatOptions?.Instructions is not null || aiContext.Instructions is not null) - { - chatOptions ??= new(); - chatOptions.Instructions = aiContext.Instructions; - } + if (chatOptions?.Instructions is not null || aiContext.Instructions is not null) + { + chatOptions ??= new(); + chatOptions.Instructions = aiContext.Instructions; } } @@ -788,13 +785,6 @@ private async Task chatOptions.ConversationId = typedSession.ConversationId; } - // When per-service-call persistence is active, set a sentinel conversation ID so that - // FunctionInvokingChatClient treats locally-persisted history the same as service-managed - // history. This prevents it from adding duplicate FunctionCallContent messages into the - // request when processing approval responses — the loaded history already contains them. - // ChatHistoryPersistingChatClient strips the sentinel before forwarding to the inner client. - chatOptions = this.SetLocalHistoryConversationIdIfNeeded(chatOptions); - // Materialize the accumulated messages once at the end of the provider pipeline, reusing the existing list if possible. List messagesList = inputMessagesForChatClient as List ?? inputMessagesForChatClient.ToList(); @@ -839,8 +829,6 @@ internal void UpdateSessionConversationId(ChatClientAgentSession session, string } } - // If we got a conversation id back from the chat client, it means that the service supports server side session storage - // so we should update the session with the new id. session.ConversationId = responseConversationId; } } @@ -849,14 +837,14 @@ internal void UpdateSessionConversationId(ChatClientAgentSession session, string /// Updates the session conversation ID at the end of an agent run. /// /// - /// When a in persist mode handles per-service-call - /// conversation ID updates, this end-of-run update is skipped. When the decorator is in mark-only - /// mode or absent, the update is performed here. When is + /// When a handles per-service-call + /// conversation ID updates, this end-of-run update is skipped. When the decorator is + /// absent, the update is performed here. When is /// (continuation token scenarios), the update is always performed. /// private void UpdateSessionConversationIdAtEndOfRun(ChatClientAgentSession session, string? responseConversationId, CancellationToken cancellationToken, bool forceUpdate = false) { - if (!forceUpdate && this.PersistsChatHistoryPerServiceCall) + if (!forceUpdate && this.SimulatesServiceStoredChatHistory) { return; } @@ -868,10 +856,9 @@ private void UpdateSessionConversationIdAtEndOfRun(ChatClientAgentSession sessio /// Notifies providers of successfully completed messages at the end of an agent run. /// /// - /// When a in persist mode handles per-service-call - /// notification, this end-of-run notification is skipped. When the decorator is in mark-only mode, - /// only the marked messages are persisted. When no decorator is present (custom stack with - /// ), all messages are persisted. + /// When a handles per-service-call + /// notification, this end-of-run notification is skipped. When no decorator is present, + /// all messages are persisted. /// When is (continuation token or /// background response scenarios), notification is always performed with all messages because /// per-service-call persistence is unreliable in these scenarios. @@ -884,19 +871,11 @@ private Task NotifyProvidersOfNewMessagesAtEndOfRunAsync( CancellationToken cancellationToken, bool forceNotify = false) { - if (!forceNotify && this.PersistsChatHistoryPerServiceCall) + if (!forceNotify && this.SimulatesServiceStoredChatHistory) { return Task.CompletedTask; } - if (!forceNotify && this.HasMarkOnlyChatHistoryPersistingClient) - { - // In mark-only mode, persist only messages that were marked by the decorator. - var markedRequestMessages = GetMarkedMessages(requestMessages); - var markedResponseMessages = GetMarkedMessages(responseMessages); - return this.NotifyProvidersOfNewMessagesAsync(session, markedRequestMessages, markedResponseMessages, chatOptions, cancellationToken); - } - return this.NotifyProvidersOfNewMessagesAsync(session, requestMessages, responseMessages, chatOptions, cancellationToken); } @@ -904,7 +883,7 @@ private Task NotifyProvidersOfNewMessagesAtEndOfRunAsync( /// Notifies providers of a failure at the end of an agent run. /// /// - /// When a in persist mode handles per-service-call + /// When a handles per-service-call /// notification (including failure), this end-of-run notification is skipped to avoid /// duplicate notification. In all other cases, failure is reported at the end of the run. /// @@ -915,7 +894,7 @@ private Task NotifyProvidersOfFailureAtEndOfRunAsync( ChatOptions? chatOptions, CancellationToken cancellationToken) { - if (this.PersistsChatHistoryPerServiceCall) + if (this.SimulatesServiceStoredChatHistory) { return Task.CompletedTask; } @@ -924,60 +903,19 @@ private Task NotifyProvidersOfFailureAtEndOfRunAsync( } /// - /// Gets a value indicating whether the agent has a - /// decorator in persist mode (not mark-only), which handles per-service-call persistence. + /// Gets a value indicating whether the agent is configured to simulate service-stored chat history. + /// When , end-of-run persistence and history loading are skipped because a + /// per-service-call decorator (such as or a + /// user-supplied equivalent) is expected to handle the history lifecycle. /// - private bool PersistsChatHistoryPerServiceCall + private bool SimulatesServiceStoredChatHistory { get { - var persistingClient = this.ChatClient.GetService(); - return persistingClient?.MarkOnly == false; + return this._agentOptions?.SimulateServiceStoredChatHistory is true; } } - /// - /// Sets the sentinel on - /// when per-service-call persistence is active and no real - /// conversation ID is present. - /// - /// - /// The (possibly new) with the sentinel set, or the original - /// if no sentinel is needed. - /// - private ChatOptions? SetLocalHistoryConversationIdIfNeeded(ChatOptions? chatOptions) - { - if (this.PersistsChatHistoryPerServiceCall && string.IsNullOrWhiteSpace(chatOptions?.ConversationId)) - { - chatOptions ??= new ChatOptions(); - chatOptions.ConversationId = ChatHistoryPersistingChatClient.LocalHistoryConversationId; - } - - return chatOptions; - } - - /// - /// Gets a value indicating whether the agent has a - /// decorator in mark-only mode, which marks messages for later persistence at the end of the run. - /// - private bool HasMarkOnlyChatHistoryPersistingClient - { - get - { - var persistingClient = this.ChatClient.GetService(); - return persistingClient?.MarkOnly == true; - } - } - - /// - /// Returns only the messages that have been marked as persisted by a in mark-only mode. - /// - private static List GetMarkedMessages(IEnumerable messages) - { - return messages.Where(m => - m.AdditionalProperties?.TryGetValue(ChatHistoryPersistingChatClient.PersistedMarkerKey, out var value) == true && value is true).ToList(); - } - /// /// Ensures that contains the resolved session. /// @@ -985,7 +923,7 @@ private static List GetMarkedMessages(IEnumerable mess /// The base class sets with the raw session parameter /// (which may be null) and restores it after each yield in streaming scenarios. After /// resolves or creates a session, we update the - /// context so the decorator always has a valid session. + /// context so the decorator always has a valid session. /// The original agent from the context is preserved to maintain the top-of-stack agent in /// decorated agent scenarios. /// @@ -1001,19 +939,19 @@ private static void EnsureRunContextHasSession(ChatClientAgentSession safeSessio /// /// Checks for potential misconfiguration when using a custom chat client stack and logs warnings. /// - private void WarnOnMissingPersistingClient() + private void WarnOnMissingServiceStoredSimulatingClient() { if (this._agentOptions?.UseProvidedChatClientAsIs is not true) { return; } - if (this._agentOptions?.PersistChatHistoryAtEndOfRun is not true) + if (this._agentOptions?.SimulateServiceStoredChatHistory is not true) { return; } - var persistingClient = this.ChatClient.GetService(); + var persistingClient = this.ChatClient.GetService(); if (persistingClient is null && this._logger.IsEnabled(LogLevel.Warning)) { var loggingAgentName = this.GetLoggingAgentName(); @@ -1023,14 +961,14 @@ private void WarnOnMissingPersistingClient() } } - private ChatHistoryProvider? ResolveChatHistoryProvider(ChatOptions? chatOptions, ChatClientAgentSession session) + private ChatHistoryProvider? ResolveChatHistoryProvider(ChatOptions? chatOptions) { - ChatHistoryProvider? provider = session.ConversationId is null ? this.ChatHistoryProvider : null; + ChatHistoryProvider? provider = chatOptions?.ConversationId is null ? this.ChatHistoryProvider : null; // If someone provided an override ChatHistoryProvider via AdditionalProperties, we should use that instead. if (chatOptions?.AdditionalProperties?.TryGetValue(out ChatHistoryProvider? overrideProvider) is true) { - if (session.ConversationId is not null && overrideProvider is not null) + if (this._agentOptions?.ThrowOnChatHistoryProviderConflict is true && string.IsNullOrWhiteSpace(chatOptions?.ConversationId) is false) { 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)}."); @@ -1055,6 +993,29 @@ private void WarnOnMissingPersistingClient() return provider; } + /// + /// Loads chat history from the resolved and prepends it to the given messages. + /// + /// + /// This method is used by both the agent (during ) and by + /// to load history before each service call. + /// + internal async Task> LoadChatHistoryAsync( + ChatClientAgentSession session, + IEnumerable messages, + ChatOptions? chatOptions, + CancellationToken cancellationToken) + { + var chatHistoryProvider = this.ResolveChatHistoryProvider(chatOptions); + if (chatHistoryProvider is null) + { + return messages; + } + + var invokingContext = new ChatHistoryProvider.InvokingContext(this, session, messages); + return await chatHistoryProvider.InvokingAsync(invokingContext, cancellationToken).ConfigureAwait(false); + } + private static ChatClientAgentContinuationToken? WrapContinuationToken(ResponseContinuationToken? continuationToken, IEnumerable? inputMessages = null, List? responseUpdates = null) { if (continuationToken is null) diff --git a/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgentLogMessages.cs b/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgentLogMessages.cs index 2a324522a4..9d6977e716 100644 --- a/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgentLogMessages.cs +++ b/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgentLogMessages.cs @@ -72,12 +72,12 @@ public static partial void LogAgentChatClientHistoryProviderConflict( /// /// Logs a warning when is - /// and is , - /// but no is found in the custom chat client stack. + /// and is , + /// but no is found in the custom chat client stack. /// [LoggerMessage( Level = LogLevel.Warning, - Message = "Agent {AgentId}/{AgentName}: PersistChatHistoryAtEndOfRun is enabled with a custom chat client stack (UseProvidedChatClientAsIs), but no ChatHistoryPersistingChatClient was found in the pipeline. All messages will be persisted at the end of the run without marking. This setup is not supported with some other features, e.g. handoffs. Consider adding a ChatHistoryPersistingChatClient to the pipeline using the UseChatHistoryPersisting extension method.")] + Message = "Agent {AgentId}/{AgentName}: SimulateServiceStoredChatHistory is enabled with a custom chat client stack (UseProvidedChatClientAsIs), but no ServiceStoredSimulatingChatClient was found in the pipeline. Chat history will not be persisted by ChatClientAgent. Consider adding a ServiceStoredSimulatingChatClient to the pipeline using the UseServiceStoredChatHistorySimulation extension method if you have not added your own persistence mechanism.")] public static partial void LogAgentChatClientMissingPersistingClient( this ILogger logger, string agentId, @@ -92,7 +92,7 @@ public static partial void LogAgentChatClientMissingPersistingClient( /// [LoggerMessage( Level = LogLevel.Warning, - Message = "Agent {AgentId}/{AgentName}: Per-service-call persistence is falling back to end-of-run persistence because the run involves background responses. Messages will be marked during the run and persisted at the end.")] + Message = "Agent {AgentId}/{AgentName}: SimulateServiceStoredChatHistory is enabled but we have to fall back to end-of-run persistence because the run involves background responses. Messages will be marked during the run and persisted at the end.")] public static partial void LogAgentChatClientBackgroundResponseFallback( this ILogger logger, string agentId, diff --git a/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgentOptions.cs b/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgentOptions.cs index 8df9112446..4387d8c5ba 100644 --- a/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgentOptions.cs +++ b/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgentOptions.cs @@ -92,54 +92,42 @@ public sealed class ChatClientAgentOptions public bool ThrowOnChatHistoryProviderConflict { get; set; } = true; /// - /// Gets or sets a value indicating whether to persist chat history only at the end of the full agent run - /// rather than after each individual service call. + /// Gets or sets a value indicating whether the should simulate + /// service-stored chat history behavior using its configured . /// /// /// - /// By default, persists request and response messages either via - /// a , or the underlying AI service's chat history storage. - /// Persistence is done immediately after each call to the AI service within the function invocation loop. - /// When storing in the underlying AI service, the session's - /// is also updated after each service call, keeping it in sync with the service-side conversation state. + /// When set to , a decorator is + /// injected between the and the leaf + /// in the chat client pipeline. This decorator takes full ownership of the chat history lifecycle: + /// it loads history from the before each service call and persists + /// new messages after each service call. It also returns a sentinel + /// on the response, causing the to treat the conversation + /// as service-managed — clearing accumulated history and not injecting duplicate + /// during approval-response processing. /// /// - /// Setting this property to causes messages to be marked during the function - /// invocation loop but persisted only at the end of the full agent run, providing atomic run semantics. - /// Updating the is likewise deferred and - /// updated only at the end of the run, consistent with atomic run semantics. - /// A decorator is inserted into the chat client pipeline - /// in mark-only mode, and the persists only the marked messages at the - /// end of the run. + /// This mode aligns the behavior of framework-managed chat history with service-stored chat history, + /// ensuring consistency in how messages are stored and loaded, including during function calling loops + /// and tool-call termination scenarios. /// /// - /// When this option is (the default), the - /// decorator persists messages and updates the - /// immediately after each service call. This may leave chat history in a state where - /// is required to start a new run if the last successful service - /// call returned . + /// When set to (the default), the handles + /// chat history persistence at the end of the full agent run via the + /// pipeline. /// /// /// This option has no effect when is . - /// When using a custom chat client stack, you can add a - /// manually via the + /// When using a custom chat client stack, you can add a + /// manually via the /// extension method. /// - /// - /// Note that when using single threaded service stored chat history, like OpenAI Conversations, - /// there is only one id, so even if the conversation id is not updated after each service call, - /// the chat history will still contain intermediate messages. Setting this property to - /// in this case will therefore have no real effect. Setting this property to when using - /// OpenAI Responses with response ids on the other hand, allows atomic run semantics, since - /// each service request produces a new response id, and if the run fails mid-loop, the session will - /// still contain the pre-run respnose id, allowing the next run to start with a clean slate. - /// /// /// /// Default is . /// [Experimental(DiagnosticIds.Experiments.AgentsAIExperiments)] - public bool PersistChatHistoryAtEndOfRun { get; set; } + public bool SimulateServiceStoredChatHistory { get; set; } /// /// Creates a new instance of with the same values as this instance. @@ -157,6 +145,6 @@ public ChatClientAgentOptions Clone() ClearOnChatHistoryProviderConflict = this.ClearOnChatHistoryProviderConflict, WarnOnChatHistoryProviderConflict = this.WarnOnChatHistoryProviderConflict, ThrowOnChatHistoryProviderConflict = this.ThrowOnChatHistoryProviderConflict, - PersistChatHistoryAtEndOfRun = this.PersistChatHistoryAtEndOfRun, + SimulateServiceStoredChatHistory = this.SimulateServiceStoredChatHistory, }; } diff --git a/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientBuilderExtensions.cs b/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientBuilderExtensions.cs index a1e8b5f8a5..68e1307eeb 100644 --- a/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientBuilderExtensions.cs +++ b/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientBuilderExtensions.cs @@ -86,25 +86,21 @@ public static ChatClientAgent BuildAIAgent( services: services); /// - /// Adds a to the chat client pipeline. + /// Adds a to the chat client pipeline. /// /// /// /// This decorator should be positioned between the and the leaf - /// in the pipeline. It intercepts service calls to either persist messages - /// immediately or mark them for later persistence, depending on the parameter. - /// - /// - /// If is set to , the - /// should be configured with set to - /// as without this combination, messages will never be persisted when using a for - /// chat history persistence. + /// in the pipeline. It simulates service-stored chat history behavior by + /// loading history before each service call, persisting after each call, and returning a sentinel + /// on the response. /// /// /// This extension method is intended for use with custom chat client stacks when /// is . /// When is (the default), - /// the automatically injects this decorator. + /// the automatically injects this decorator when + /// is . /// /// /// This decorator only works within the context of a running and will throw an @@ -112,18 +108,10 @@ public static ChatClientAgent BuildAIAgent( /// /// /// The to add the decorator to. - /// - /// When , messages are marked with metadata but not persisted immediately, - /// and the session's is not updated. - /// The will persist only the marked messages and update the - /// conversation ID at the end of the run. - /// When (the default), messages are persisted and the conversation ID - /// is updated immediately after each service call. - /// /// The for chaining. [Experimental(DiagnosticIds.Experiments.AgentsAIExperiments)] - public static ChatClientBuilder UseChatHistoryPersisting(this ChatClientBuilder builder, bool markOnly = false) + public static ChatClientBuilder UseServiceStoredChatHistorySimulation(this ChatClientBuilder builder) { - return builder.Use(innerClient => new ChatHistoryPersistingChatClient(innerClient, markOnly)); + return builder.Use(innerClient => new ServiceStoredSimulatingChatClient(innerClient)); } } diff --git a/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientExtensions.cs b/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientExtensions.cs index fffac628a6..5251314766 100644 --- a/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientExtensions.cs +++ b/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientExtensions.cs @@ -63,14 +63,17 @@ internal static IChatClient WithDefaultAgentMiddleware(this IChatClient chatClie }); } - // ChatHistoryPersistingChatClient is registered after FunctionInvokingChatClient so that it sits - // between FIC and the leaf client. ChatClientBuilder.Build applies factories in reverse order, - // making the first Use() call outermost. By adding our decorator second, the resulting pipeline is: - // FunctionInvokingChatClient → ChatHistoryPersistingChatClient → leaf IChatClient - // This allows the decorator to persist messages after each individual service call within - // FIC's function invocation loop, or to mark them for later persistence at the end of the run. - bool markOnly = options?.PersistChatHistoryAtEndOfRun is true; - chatBuilder.Use(innerClient => new ChatHistoryPersistingChatClient(innerClient, markOnly)); + // ServiceStoredSimulatingChatClient is only injected when SimulateServiceStoredChatHistory is enabled. + // It is registered after FunctionInvokingChatClient so that it sits between FIC and the leaf client. + // ChatClientBuilder.Build applies factories in reverse order, making the first Use() call outermost. + // By adding our decorator second, the resulting pipeline is: + // FunctionInvokingChatClient → ServiceStoredSimulatingChatClient → leaf IChatClient + // This allows the decorator to simulate service-stored chat history by loading history before + // each service call, persisting after each call, and returning a sentinel ConversationId. + if (options?.SimulateServiceStoredChatHistory is true) + { + chatBuilder.Use(innerClient => new ServiceStoredSimulatingChatClient(innerClient)); + } var agentChatClient = chatBuilder.Build(services); diff --git a/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatHistoryPersistingChatClient.cs b/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatHistoryPersistingChatClient.cs deleted file mode 100644 index e733b778eb..0000000000 --- a/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatHistoryPersistingChatClient.cs +++ /dev/null @@ -1,351 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. - -using System; -using System.Collections.Generic; -using System.Linq; -using System.Runtime.CompilerServices; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.Extensions.AI; - -namespace Microsoft.Agents.AI; - -/// -/// A delegating chat client that notifies and -/// instances of request and response messages after each individual call to the inner chat client, -/// or marks messages for later persistence depending on the configured mode. -/// -/// -/// -/// This decorator is intended to operate between the and the leaf -/// in a pipeline. -/// -/// -/// In persist mode (the default), it ensures that providers are notified and the session's -/// is updated after each service call, so that -/// intermediate messages (e.g., tool calls and results) are saved even if the process is interrupted -/// mid-loop. -/// -/// -/// In mark-only mode ( is ), it marks messages with metadata -/// but does not notify providers or update the . -/// Both are deferred to the at the end of the run, providing atomic -/// run semantics. -/// -/// -/// This chat client must be used within the context of a running . It retrieves the -/// current agent and session from , which is set automatically when an agent's -/// or -/// -/// method is called. The ensures the run context always contains a resolved session, -/// even when the caller passes null. An is thrown if no run context is -/// available or if the agent is not a . -/// -/// -internal sealed class ChatHistoryPersistingChatClient : DelegatingChatClient -{ - /// - /// The key used in and - /// to mark messages and their content as already persisted to chat history. - /// - internal const string PersistedMarkerKey = "_chatHistoryPersisted"; - - /// - /// A sentinel value set on by - /// when per-service-call persistence is active and no real conversation ID exists. - /// - /// - /// - /// This signals to that the chat history is being managed - /// externally (by this decorator), which prevents it from adding duplicate - /// messages into the request during approval-response processing. Without this sentinel, - /// would reconstruct function-call messages from approval - /// responses and append them to the original messages — but the loaded history already contains - /// those same function calls, causing duplicate tool-call entries that the model rejects. - /// - /// - /// This decorator strips the sentinel before forwarding requests to the inner client, so the - /// underlying model never sees it. - /// - /// - internal const string LocalHistoryConversationId = "_agent_local_history"; - - /// - /// Initializes a new instance of the class. - /// - /// The underlying chat client that will handle the core operations. - /// - /// When , messages are marked with metadata but not persisted immediately, - /// and the session's is not updated. - /// The will persist only the marked messages and update the - /// conversation ID at the end of the run. - /// When (the default), messages are persisted and the conversation ID - /// is updated immediately after each service call. - /// - public ChatHistoryPersistingChatClient(IChatClient innerClient, bool markOnly = false) - : base(innerClient) - { - this.MarkOnly = markOnly; - } - - /// - /// Gets a value indicating whether this decorator is in mark-only mode. - /// - /// - /// When , messages are marked with metadata but not persisted immediately, - /// and the session's is not updated. - /// Both are deferred to the at the end of the run. - /// When , messages are persisted and the conversation ID is updated - /// after each service call. - /// - public bool MarkOnly { get; } - - /// - public override async Task GetResponseAsync( - IEnumerable messages, - ChatOptions? options = null, - CancellationToken cancellationToken = default) - { - var (agent, session) = GetRequiredAgentAndSession(); - options = StripLocalHistoryConversationId(options); - - ChatResponse response; - try - { - response = await base.GetResponseAsync(messages, options, cancellationToken).ConfigureAwait(false); - } - catch (Exception ex) - { - var newRequestMessagesOnFailure = GetNewRequestMessages(messages); - await agent.NotifyProvidersOfFailureAsync(session, ex, newRequestMessagesOnFailure, options, cancellationToken).ConfigureAwait(false); - throw; - } - - var newRequestMessages = GetNewRequestMessages(messages); - - if (this.ShouldDeferPersistence(options)) - { - // In mark-only mode or when resuming from a continuation token, just mark messages - // for later persistence by ChatClientAgent. Conversation ID and provider notification - // are deferred to end-of-run. For continuation tokens, the end-of-run handler needs - // to send the combined data from both the previous and current runs. - MarkAsPersisted(newRequestMessages); - MarkAsPersisted(response.Messages); - } - else - { - // In persist mode, persist immediately and update conversation ID. - agent.UpdateSessionConversationId(session, response.ConversationId, cancellationToken); - await agent.NotifyProvidersOfNewMessagesAsync(session, newRequestMessages, response.Messages, options, cancellationToken).ConfigureAwait(false); - MarkAsPersisted(newRequestMessages); - MarkAsPersisted(response.Messages); - } - - return response; - } - - /// - public override async IAsyncEnumerable GetStreamingResponseAsync( - IEnumerable messages, - ChatOptions? options = null, - [EnumeratorCancellation] CancellationToken cancellationToken = default) - { - var (agent, session) = GetRequiredAgentAndSession(); - options = StripLocalHistoryConversationId(options); - - List responseUpdates = []; - - IAsyncEnumerator enumerator; - try - { - enumerator = base.GetStreamingResponseAsync(messages, options, cancellationToken).GetAsyncEnumerator(cancellationToken); - } - catch (Exception ex) - { - var newRequestMessagesOnFailure = GetNewRequestMessages(messages); - await agent.NotifyProvidersOfFailureAsync(session, ex, newRequestMessagesOnFailure, options, cancellationToken).ConfigureAwait(false); - throw; - } - - bool hasUpdates; - try - { - hasUpdates = await enumerator.MoveNextAsync().ConfigureAwait(false); - } - catch (Exception ex) - { - var newRequestMessagesOnFailure = GetNewRequestMessages(messages); - await agent.NotifyProvidersOfFailureAsync(session, ex, newRequestMessagesOnFailure, options, cancellationToken).ConfigureAwait(false); - throw; - } - - while (hasUpdates) - { - var update = enumerator.Current; - responseUpdates.Add(update); - yield return update; - - try - { - hasUpdates = await enumerator.MoveNextAsync().ConfigureAwait(false); - } - catch (Exception ex) - { - var newRequestMessagesOnFailure = GetNewRequestMessages(messages); - await agent.NotifyProvidersOfFailureAsync(session, ex, newRequestMessagesOnFailure, options, cancellationToken).ConfigureAwait(false); - throw; - } - } - - var chatResponse = responseUpdates.ToChatResponse(); - var newRequestMessages = GetNewRequestMessages(messages); - - if (this.ShouldDeferPersistence(options)) - { - // In mark-only mode or when resuming from a continuation token, just mark messages - // for later persistence by ChatClientAgent. Conversation ID and provider notification - // are deferred to end-of-run. For continuation tokens, the end-of-run handler needs - // to send the combined data from both the previous and current runs. - MarkAsPersisted(newRequestMessages); - MarkAsPersisted(chatResponse.Messages); - } - else - { - // In persist mode, persist immediately and update conversation ID. - agent.UpdateSessionConversationId(session, chatResponse.ConversationId, cancellationToken); - await agent.NotifyProvidersOfNewMessagesAsync(session, newRequestMessages, chatResponse.Messages, options, cancellationToken).ConfigureAwait(false); - MarkAsPersisted(newRequestMessages); - MarkAsPersisted(chatResponse.Messages); - } - } - - /// - /// Gets the current and from the run context. - /// - private static (ChatClientAgent Agent, ChatClientAgentSession Session) GetRequiredAgentAndSession() - { - var runContext = AIAgent.CurrentRunContext - ?? throw new InvalidOperationException( - $"{nameof(ChatHistoryPersistingChatClient)} can only be used within the context of a running AIAgent. " + - "Ensure that the chat client is being invoked as part of an AIAgent.RunAsync or AIAgent.RunStreamingAsync call."); - - var chatClientAgent = runContext.Agent.GetService() - ?? throw new InvalidOperationException( - $"{nameof(ChatHistoryPersistingChatClient)} can only be used with a {nameof(ChatClientAgent)}. " + - $"The current agent is of type '{runContext.Agent.GetType().Name}'."); - - if (runContext.Session is not ChatClientAgentSession chatClientAgentSession) - { - throw new InvalidOperationException( - $"{nameof(ChatHistoryPersistingChatClient)} requires a {nameof(ChatClientAgentSession)}. " + - $"The current session is of type '{runContext.Session?.GetType().Name ?? "null"}'."); - } - - return (chatClientAgent, chatClientAgentSession); - } - - /// - /// Determines whether persistence should be deferred to end-of-run instead of happening immediately. - /// - /// - /// when in mode, when the call is resuming from - /// a continuation token (since the end-of-run handler needs to combine data from the previous - /// and current runs), or when background responses are allowed (since the caller may stop - /// consuming the stream mid-run, preventing the post-stream persistence code from executing). - /// - private bool ShouldDeferPersistence(ChatOptions? options) - { - return this.MarkOnly || options?.ContinuationToken is not null || options?.AllowBackgroundResponses is true; - } - - /// - /// Returns only the request messages that have not yet been persisted to chat history. - /// - /// - /// A message is considered already persisted if any of the following is true: - /// - /// It has the in its . - /// It has an of - /// (indicating it was loaded from chat history and does not need to be re-persisted). - /// It has and all of its items have the - /// in their . This handles the - /// streaming case where reconstructs objects - /// independently via ToChatResponse(), producing different object references that share the same - /// underlying instances. - /// - /// - /// A list of request messages that have not yet been persisted. - /// The full set of request messages to filter. - private static List GetNewRequestMessages(IEnumerable messages) - { - return messages.Where(m => !IsAlreadyPersisted(m)).ToList(); - } - - /// - /// Determines whether a message has already been persisted to chat history by this decorator. - /// - private static bool IsAlreadyPersisted(ChatMessage message) - { - if (message.AdditionalProperties?.TryGetValue(PersistedMarkerKey, out var value) == true && value is true) - { - return true; - } - - if (message.GetAgentRequestMessageSourceType() == AgentRequestMessageSourceType.ChatHistory) - { - return true; - } - - // In streaming mode, FunctionInvokingChatClient reconstructs ChatMessage objects via ToChatResponse() - // independently, producing different ChatMessage instances. However, the underlying AIContent objects - // (e.g., FunctionCallContent, FunctionResultContent) are shared references. Checking for markers on - // AIContent handles dedup in this case. - if (message.Contents.Count > 0 && message.Contents.All(c => c.AdditionalProperties?.TryGetValue(PersistedMarkerKey, out var value) == true && value is true)) - { - return true; - } - - return false; - } - - /// - /// Marks the given messages as persisted by setting a marker on both the - /// and each of its items. - /// - /// - /// Both levels are marked because may reconstruct - /// objects in streaming mode (losing the message-level marker), - /// but the references are shared and retain their markers. - /// - /// The messages to mark as persisted. - private static void MarkAsPersisted(IEnumerable messages) - { - foreach (var message in messages) - { - message.AdditionalProperties ??= new(); - message.AdditionalProperties[PersistedMarkerKey] = true; - - foreach (var content in message.Contents) - { - content.AdditionalProperties ??= new(); - content.AdditionalProperties[PersistedMarkerKey] = true; - } - } - } - - /// - /// If the carry the sentinel, - /// returns a clone with the conversation ID cleared so the inner client never sees it. - /// Otherwise returns the original unchanged. - /// - private static ChatOptions? StripLocalHistoryConversationId(ChatOptions? options) - { - if (options?.ConversationId == LocalHistoryConversationId) - { - options = options.Clone(); - options.ConversationId = null; - } - - return options; - } -} diff --git a/dotnet/src/Microsoft.Agents.AI/ChatClient/ServiceStoredSimulatingChatClient.cs b/dotnet/src/Microsoft.Agents.AI/ChatClient/ServiceStoredSimulatingChatClient.cs new file mode 100644 index 0000000000..710ffad9be --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI/ChatClient/ServiceStoredSimulatingChatClient.cs @@ -0,0 +1,239 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Runtime.CompilerServices; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.AI; + +namespace Microsoft.Agents.AI; + +/// +/// A delegating chat client that simulates service-stored chat history behavior using +/// framework-managed instances. +/// +/// +/// +/// This decorator is intended to operate between the and the leaf +/// in a pipeline. +/// +/// +/// Before each service call, it loads chat history from the agent's +/// and prepends it to the request messages. After each successful service call, it persists +/// new request and response messages to the provider. It also returns a sentinel +/// on the response so that the +/// treats the conversation as service-managed — +/// clearing accumulated history between iterations and not injecting duplicate +/// during approval-response processing. +/// +/// +/// This chat client must be used within the context of a running . It retrieves the +/// current agent and session from , which is set automatically when an agent's +/// or +/// +/// method is called. The ensures the run context always contains a resolved session, +/// even when the caller passes null. An is thrown if no run context is +/// available or if the agent is not a . +/// +/// +internal sealed class ServiceStoredSimulatingChatClient : DelegatingChatClient +{ + /// + /// A sentinel value returned on to signal + /// that chat history is being managed downstream. + /// + /// + /// + /// When sees a non-null , + /// it treats the conversation as service-managed: it clears accumulated history between + /// iterations (via FixupHistories) and does not inject + /// into the request during approval-response processing (via ProcessFunctionApprovalResponses). + /// + /// + /// This decorator strips the sentinel from on incoming + /// requests before forwarding to the inner client, so the underlying model never sees it. + /// + /// + internal const string LocalHistoryConversationId = "_agent_local_chat_history"; + + /// + /// Initializes a new instance of the class. + /// + /// The underlying chat client that will handle the core operations. + public ServiceStoredSimulatingChatClient(IChatClient innerClient) + : base(innerClient) + { + } + + /// + public override async Task GetResponseAsync( + IEnumerable messages, + ChatOptions? options = null, + CancellationToken cancellationToken = default) + { + var (agent, session) = GetRequiredAgentAndSession(); + options = StripLocalHistoryConversationId(options); + + // Load history and prepend it to the messages. + var allMessages = await agent.LoadChatHistoryAsync(session, messages, options, cancellationToken).ConfigureAwait(false); + + ChatResponse response; + try + { + response = await base.GetResponseAsync(allMessages, options, cancellationToken).ConfigureAwait(false); + } + catch (Exception ex) + { + var newRequestMessages = GetNewRequestMessages(allMessages); + await agent.NotifyProvidersOfFailureAsync(session, ex, newRequestMessages, options, cancellationToken).ConfigureAwait(false); + throw; + } + + var newMessages = GetNewRequestMessages(allMessages); + + // Persist immediately after each service call. + await agent.NotifyProvidersOfNewMessagesAsync(session, newMessages, response.Messages, options, cancellationToken).ConfigureAwait(false); + + // Set the sentinel ConversationId on the response and session. + SetSentinelConversationId(response, session); + + return response; + } + + /// + public override async IAsyncEnumerable GetStreamingResponseAsync( + IEnumerable messages, + ChatOptions? options = null, + [EnumeratorCancellation] CancellationToken cancellationToken = default) + { + var (agent, session) = GetRequiredAgentAndSession(); + options = StripLocalHistoryConversationId(options); + + // Load history and prepend it to the messages. + var allMessages = await agent.LoadChatHistoryAsync(session, messages, options, cancellationToken).ConfigureAwait(false); + + List responseUpdates = []; + + IAsyncEnumerator enumerator; + try + { + enumerator = base.GetStreamingResponseAsync(allMessages, options, cancellationToken).GetAsyncEnumerator(cancellationToken); + } + catch (Exception ex) + { + var newRequestMessagesOnFailure = GetNewRequestMessages(allMessages); + await agent.NotifyProvidersOfFailureAsync(session, ex, newRequestMessagesOnFailure, options, cancellationToken).ConfigureAwait(false); + throw; + } + + bool hasUpdates; + try + { + hasUpdates = await enumerator.MoveNextAsync().ConfigureAwait(false); + } + catch (Exception ex) + { + var newRequestMessagesOnFailure = GetNewRequestMessages(allMessages); + await agent.NotifyProvidersOfFailureAsync(session, ex, newRequestMessagesOnFailure, options, cancellationToken).ConfigureAwait(false); + throw; + } + + while (hasUpdates) + { + var update = enumerator.Current; + responseUpdates.Add(update); + update.ConversationId = LocalHistoryConversationId; // Set the sentinel ConversationId on each update for the streaming case. + yield return update; + + try + { + hasUpdates = await enumerator.MoveNextAsync().ConfigureAwait(false); + } + catch (Exception ex) + { + var newRequestMessagesOnFailure = GetNewRequestMessages(allMessages); + await agent.NotifyProvidersOfFailureAsync(session, ex, newRequestMessagesOnFailure, options, cancellationToken).ConfigureAwait(false); + throw; + } + } + + var chatResponse = responseUpdates.ToChatResponse(); + var newMessages = GetNewRequestMessages(allMessages); + + // Persist immediately after each service call. + await agent.NotifyProvidersOfNewMessagesAsync(session, newMessages, chatResponse.Messages, options, cancellationToken).ConfigureAwait(false); + + // Set the sentinel ConversationId on the session. For streaming, set it on the last update. + session.ConversationId = LocalHistoryConversationId; + } + + /// + /// Sets the sentinel on the response and session + /// so that treats the conversation as service-managed. + /// + private static void SetSentinelConversationId(ChatResponse response, ChatClientAgentSession session) + { + response.ConversationId = LocalHistoryConversationId; + session.ConversationId = LocalHistoryConversationId; + } + + /// + /// Gets the current and from the run context. + /// + private static (ChatClientAgent Agent, ChatClientAgentSession Session) GetRequiredAgentAndSession() + { + var runContext = AIAgent.CurrentRunContext + ?? throw new InvalidOperationException( + $"{nameof(ServiceStoredSimulatingChatClient)} can only be used within the context of a running AIAgent. " + + "Ensure that the chat client is being invoked as part of an AIAgent.RunAsync or AIAgent.RunStreamingAsync call."); + + var chatClientAgent = runContext.Agent.GetService() + ?? throw new InvalidOperationException( + $"{nameof(ServiceStoredSimulatingChatClient)} can only be used with a {nameof(ChatClientAgent)}. " + + $"The current agent is of type '{runContext.Agent.GetType().Name}'."); + + if (runContext.Session is not ChatClientAgentSession chatClientAgentSession) + { + throw new InvalidOperationException( + $"{nameof(ServiceStoredSimulatingChatClient)} requires a {nameof(ChatClientAgentSession)}. " + + $"The current session is of type '{runContext.Session?.GetType().Name ?? "null"}'."); + } + + return (chatClientAgent, chatClientAgentSession); + } + + /// + /// Returns only the request messages that have not been loaded from chat history. + /// + /// + /// Messages loaded by the are tagged with + /// and should not be re-persisted. + /// Because treats the conversation as service-managed + /// (via the sentinel ), it clears accumulated history + /// between iterations, so only genuinely new messages arrive here. + /// + /// The full set of request messages to filter. + /// A list of request messages that need to be persisted. + private static List GetNewRequestMessages(IEnumerable messages) + { + return messages.Where(m => m.GetAgentRequestMessageSourceType() != AgentRequestMessageSourceType.ChatHistory).ToList(); + } + + /// + /// If the carry the sentinel, + /// returns a clone with the conversation ID cleared so the inner client never sees it. + /// Otherwise returns the original unchanged. + /// + private static ChatOptions? StripLocalHistoryConversationId(ChatOptions? options) + { + if (options?.ConversationId == LocalHistoryConversationId) + { + options = options.Clone(); + options.ConversationId = null; + } + + return options; + } +} diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgentTestHelper.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgentTestHelper.cs index a3d2bd0c6a..b3296a1306 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgentTestHelper.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgentTestHelper.cs @@ -14,7 +14,7 @@ namespace Microsoft.Agents.AI.UnitTests; /// /// Shared test helper for integration tests that verify -/// end-to-end behavior with and +/// end-to-end behavior with and /// . /// internal static class ChatClientAgentTestHelper diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgentTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgentTests.cs index 7241ca763e..d5e7102472 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgentTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgentTests.cs @@ -379,12 +379,10 @@ public async Task RunAsyncPassesChatOptionsWhenUsingChatClientAgentRunOptionsAsy } /// - /// Verify that RunAsync passes ChatOptions with null ConversationId when using regular AgentRunOptions. - /// When per-service-call persistence is active (default), the sentinel conversation ID is set on ChatOptions - /// and then stripped by ChatHistoryPersistingChatClient before reaching the inner client. + /// Verify that RunAsync passes null ChatOptions when using regular AgentRunOptions. /// [Fact] - public async Task RunAsyncPassesChatOptionsWithNullConversationIdWhenUsingRegularAgentRunOptionsAsync() + public async Task RunAsyncPassesNullChatOptionsWhenUsingRegularAgentRunOptionsAsync() { // Arrange ChatOptions? capturedOptions = null; @@ -403,9 +401,8 @@ public async Task RunAsyncPassesChatOptionsWithNullConversationIdWhenUsingRegula // Act await agent.RunAsync([new(ChatRole.User, "test")], options: runOptions); - // Assert — the inner client receives ChatOptions with null ConversationId (sentinel was stripped) - Assert.NotNull(capturedOptions); - Assert.Null(capturedOptions!.ConversationId); + // Assert + Assert.Null(capturedOptions); } /// diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgent_ApprovalsTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgent_ApprovalsTests.cs index 6300942c9d..f3550359dc 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgent_ApprovalsTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgent_ApprovalsTests.cs @@ -9,7 +9,7 @@ namespace Microsoft.Agents.AI.UnitTests; /// /// Contains unit tests that verify the end-to-end approval flow behavior of the -/// class with , +/// class with , /// ensuring that chat history is correctly persisted across multi-turn approval interactions. /// public class ChatClientAgent_ApprovalsTests @@ -48,7 +48,7 @@ public async Task RunAsync_ApprovalRequired_PerServiceCallPersistence_PersistsCo agentOptions: new() { ChatOptions = new() { Tools = [approvalTool] }, - PersistChatHistoryAtEndOfRun = false, + SimulateServiceStoredChatHistory = true, }, callIndex: callIndex, capturedInputs: capturedInputs); @@ -123,7 +123,6 @@ public async Task RunAsync_ApprovalRequired_EndOfRunPersistence_PersistsCorrectH agentOptions: new() { ChatOptions = new() { Tools = [approvalTool] }, - PersistChatHistoryAtEndOfRun = true, }, callIndex: callIndex, capturedInputs: capturedInputs); @@ -150,8 +149,10 @@ public async Task RunAsync_ApprovalRequired_EndOfRunPersistence_PersistsCorrectH expectedHistory: [ // End-of-run persistence retains the approval request from Turn 1 + // and the approval response from Turn 2 new(ChatRole.User, TextContains: "What's the weather?"), new(ChatRole.Assistant, ContentTypes: [typeof(ToolApprovalRequestContent)]), + new(ChatRole.User, ContentTypes: [typeof(ToolApprovalResponseContent)]), new(ChatRole.Assistant, ContentTypes: [typeof(FunctionCallContent)]), new(ChatRole.Tool, ContentTypes: [typeof(FunctionResultContent)]), new(ChatRole.Assistant, TextContains: "sunny and 22°C"), @@ -196,7 +197,6 @@ public async Task RunAsync_ApprovalRequired_ServiceStoredHistory_CompletesWithou agentOptions: new() { ChatOptions = new() { Tools = [approvalTool] }, - PersistChatHistoryAtEndOfRun = false, }, callIndex: callIndex, capturedInputs: capturedInputs); @@ -260,7 +260,7 @@ public async Task RunAsync_ApprovalRejected_PersistsRejectionInHistoryAsync() agentOptions: new() { ChatOptions = new() { Tools = [approvalTool] }, - PersistChatHistoryAtEndOfRun = false, + SimulateServiceStoredChatHistory = true, }, callIndex: callIndex, capturedInputs: capturedInputs); diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgent_ChatHistoryManagementTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgent_ChatHistoryManagementTests.cs index 3e54dbc06e..83bdc2b2a2 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgent_ChatHistoryManagementTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgent_ChatHistoryManagementTests.cs @@ -520,7 +520,7 @@ await ChatClientAgentTestHelper.RunAsync( agentOptions: new() { ChatOptions = new() { Instructions = "Be helpful" }, - PersistChatHistoryAtEndOfRun = false, + SimulateServiceStoredChatHistory = true, }, expectedServiceCallCount: 1, expectedHistory: @@ -554,7 +554,7 @@ await ChatClientAgentTestHelper.RunAsync( agentOptions: new() { ChatOptions = new() { Tools = [tool] }, - PersistChatHistoryAtEndOfRun = false, + SimulateServiceStoredChatHistory = true, }, expectedServiceCallCount: 2, expectedHistory: @@ -583,7 +583,6 @@ await ChatClientAgentTestHelper.RunAsync( agentOptions: new() { ChatOptions = new() { Instructions = "Be helpful" }, - PersistChatHistoryAtEndOfRun = true, }, expectedServiceCallCount: 1, expectedHistory: @@ -615,7 +614,6 @@ await ChatClientAgentTestHelper.RunAsync( agentOptions: new() { ChatOptions = new() { Tools = [tool] }, - PersistChatHistoryAtEndOfRun = true, }, expectedServiceCallCount: 2, expectedHistory: @@ -644,7 +642,6 @@ public async Task RunAsync_ServiceStoredHistory_SetsConversationIdAndCompletesWi agentOptions: new() { ChatOptions = new() { Instructions = "Be helpful" }, - PersistChatHistoryAtEndOfRun = false, }, expectedServiceCallCount: 1); diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgent_ChatOptionsMergingTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgent_ChatOptionsMergingTests.cs index 28d38ea36a..e4df863ce0 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgent_ChatOptionsMergingTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgent_ChatOptionsMergingTests.cs @@ -176,12 +176,11 @@ public async Task ChatOptionsMergingPrioritizesRequestOptionsOverAgentOptionsAsy } /// - /// Verify that ChatOptions merging returns a non-null ChatOptions instance with null ConversationId - /// when both agent and request have no ChatOptions. The sentinel conversation ID is set for - /// per-service-call persistence and stripped before reaching the inner client. + /// Verify that when both agent and request have no ChatOptions, the inner client + /// receives null options. /// [Fact] - public async Task ChatOptionsMergingReturnsChatOptionsWithNullConversationIdWhenBothAgentAndRequestHaveNoneAsync() + public async Task ChatOptionsMergingReturnsNullChatOptionsWhenBothAgentAndRequestHaveNoneAsync() { // Arrange Mock mockService = new(); @@ -201,9 +200,8 @@ public async Task ChatOptionsMergingReturnsChatOptionsWithNullConversationIdWhen // Act await agent.RunAsync(messages); - // Assert — ChatOptions is non-null because the sentinel was set, but ConversationId is null (stripped) - Assert.NotNull(capturedChatOptions); - Assert.Null(capturedChatOptions!.ConversationId); + // Assert + Assert.Null(capturedChatOptions); } /// diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatHistoryPersistingChatClientTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ServiceStoredSimulatingChatClientTests.cs similarity index 87% rename from dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatHistoryPersistingChatClientTests.cs rename to dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ServiceStoredSimulatingChatClientTests.cs index e7f91ab5d7..56329911c6 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatHistoryPersistingChatClientTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ServiceStoredSimulatingChatClientTests.cs @@ -13,15 +13,15 @@ namespace Microsoft.Agents.AI.UnitTests; /// -/// Contains unit tests for the decorator, +/// Contains unit tests for the decorator, /// verifying that it persists messages via the after each /// individual service call by default, or marks messages for end-of-run persistence when the -/// option is enabled. +/// option is enabled. /// -public class ChatHistoryPersistingChatClientTests +public class ServiceStoredSimulatingChatClientTests { /// - /// Verifies that by default (PersistChatHistoryAtEndOfRun is false), + /// Verifies that by default (SimulateServiceStoredChatHistory is false), /// the ChatHistoryProvider receives messages after a successful non-streaming call. /// [Fact] @@ -50,7 +50,7 @@ public async Task RunAsync_PersistsMessagesPerServiceCall_ByDefaultAsync() ChatClientAgent agent = new(mockService.Object, options: new() { ChatHistoryProvider = mockChatHistoryProvider.Object, - PersistChatHistoryAtEndOfRun = false, + SimulateServiceStoredChatHistory = true, }); // Act @@ -97,7 +97,7 @@ public async Task RunAsync_PersistsMessagesAtEndOfRun_WhenOptionEnabledAsync() ChatClientAgent agent = new(mockService.Object, options: new() { ChatHistoryProvider = mockChatHistoryProvider.Object, - PersistChatHistoryAtEndOfRun = true, + SimulateServiceStoredChatHistory = true, }); // Act @@ -145,7 +145,7 @@ public async Task RunAsync_NotifiesProviderOfFailure_WhenPerServiceCallPersisten ChatClientAgent agent = new(mockService.Object, options: new() { ChatHistoryProvider = mockChatHistoryProvider.Object, - PersistChatHistoryAtEndOfRun = false, + SimulateServiceStoredChatHistory = true, }); // Act @@ -163,11 +163,10 @@ public async Task RunAsync_NotifiesProviderOfFailure_WhenPerServiceCallPersisten } /// - /// Verifies that the decorator is injected in persist mode by default - /// and can be discovered via GetService. + /// Verifies that the decorator is NOT injected by default (SimulateServiceStoredChatHistory is false). /// [Fact] - public void ChatClient_ContainsDecorator_InPersistMode_ByDefault() + public void ChatClient_DoesNotContainDecorator_ByDefault() { // Arrange Mock mockService = new(); @@ -176,16 +175,15 @@ public void ChatClient_ContainsDecorator_InPersistMode_ByDefault() ChatClientAgent agent = new(mockService.Object, options: new()); // Assert - var decorator = agent.ChatClient.GetService(); - Assert.NotNull(decorator); - Assert.False(decorator.MarkOnly); + var decorator = agent.ChatClient.GetService(); + Assert.Null(decorator); } /// - /// Verifies that the decorator is injected in mark-only mode when PersistChatHistoryAtEndOfRun is true. + /// Verifies that the decorator is injected when SimulateServiceStoredChatHistory is true. /// [Fact] - public void ChatClient_ContainsDecorator_InMarkOnlyMode_WhenPersistAtEndOfRun() + public void ChatClient_ContainsDecorator_WhenSimulateServiceStoredChatHistory() { // Arrange Mock mockService = new(); @@ -193,13 +191,12 @@ public void ChatClient_ContainsDecorator_InMarkOnlyMode_WhenPersistAtEndOfRun() // Act ChatClientAgent agent = new(mockService.Object, options: new() { - PersistChatHistoryAtEndOfRun = true, + SimulateServiceStoredChatHistory = true, }); // Assert - var decorator = agent.ChatClient.GetService(); + var decorator = agent.ChatClient.GetService(); Assert.NotNull(decorator); - Assert.True(decorator.MarkOnly); } /// @@ -218,27 +215,27 @@ public void ChatClient_DoesNotContainDecorator_WhenUseProvidedChatClientAsIs() }); // Assert - var decorator = agent.ChatClient.GetService(); + var decorator = agent.ChatClient.GetService(); Assert.Null(decorator); } /// - /// Verifies that the PersistChatHistoryAtEndOfRun option is included in Clone(). + /// Verifies that the SimulateServiceStoredChatHistory option is included in Clone(). /// [Fact] - public void ChatClientAgentOptions_Clone_IncludesPersistChatHistoryAtEndOfRun() + public void ChatClientAgentOptions_Clone_IncludesSimulateServiceStoredChatHistory() { // Arrange var options = new ChatClientAgentOptions { - PersistChatHistoryAtEndOfRun = true, + SimulateServiceStoredChatHistory = true, }; // Act var cloned = options.Clone(); // Assert - Assert.True(cloned.PersistChatHistoryAtEndOfRun); + Assert.True(cloned.SimulateServiceStoredChatHistory); } /// @@ -292,7 +289,7 @@ public async Task RunAsync_PersistsPerServiceCall_DuringFunctionInvocationLoopAs { ChatOptions = new() { Tools = [tool] }, ChatHistoryProvider = mockChatHistoryProvider.Object, - PersistChatHistoryAtEndOfRun = false, + SimulateServiceStoredChatHistory = true, }, services: new ServiceCollection().BuildServiceProvider()); // Act @@ -361,7 +358,7 @@ public async Task RunStreamingAsync_PersistsMessagesPerServiceCall_ByDefaultAsyn ChatClientAgent agent = new(mockService.Object, options: new() { ChatHistoryProvider = mockChatHistoryProvider.Object, - PersistChatHistoryAtEndOfRun = false, + SimulateServiceStoredChatHistory = true, }); // Act @@ -410,7 +407,7 @@ public async Task RunAsync_NotifiesAIContextProviders_ByDefaultAsync() ChatClientAgent agent = new(mockService.Object, options: new() { AIContextProviders = [mockContextProvider.Object], - PersistChatHistoryAtEndOfRun = false, + SimulateServiceStoredChatHistory = true, }); // Act @@ -457,7 +454,7 @@ public async Task RunAsync_NotifiesAIContextProvidersOfFailure_ByDefaultAsync() ChatClientAgent agent = new(mockService.Object, options: new() { AIContextProviders = [mockContextProvider.Object], - PersistChatHistoryAtEndOfRun = false, + SimulateServiceStoredChatHistory = true, }); // Act @@ -516,7 +513,7 @@ public async Task RunAsync_NotifiesBothProviders_ByDefaultAsync() { ChatHistoryProvider = mockChatHistoryProvider.Object, AIContextProviders = [mockContextProvider.Object], - PersistChatHistoryAtEndOfRun = false, + SimulateServiceStoredChatHistory = true, }); // Act @@ -590,7 +587,7 @@ public async Task RunAsync_DoesNotReNotifyResponseMessagesAsRequestMessages_Duri { ChatOptions = new() { Tools = [tool] }, ChatHistoryProvider = mockChatHistoryProvider.Object, - PersistChatHistoryAtEndOfRun = false, + SimulateServiceStoredChatHistory = true, }, services: new ServiceCollection().BuildServiceProvider()); // Act @@ -655,7 +652,7 @@ public async Task RunAsync_DeduplicatesRequestMessages_OnFailureDuringFicLoopAsy { ChatOptions = new() { Tools = [tool] }, ChatHistoryProvider = mockChatHistoryProvider.Object, - PersistChatHistoryAtEndOfRun = false, + SimulateServiceStoredChatHistory = true, }, services: new ServiceCollection().BuildServiceProvider()); // Act @@ -680,52 +677,12 @@ await Assert.ThrowsAsync(() => /// Verifies that after a successful run with per-service-call persistence, the notified /// messages are stamped with the persisted marker so they are not re-notified. /// - [Fact] - public async Task RunAsync_MarksNotifiedMessages_WithPersistedMarkerAsync() - { - // Arrange - Mock mockService = new(); - mockService.Setup( - s => s.GetResponseAsync( - It.IsAny>(), - It.IsAny(), - It.IsAny())).ReturnsAsync(new ChatResponse([new(ChatRole.Assistant, "response")])); - - Mock mockChatHistoryProvider = new(null, null, null); - mockChatHistoryProvider.SetupGet(p => p.StateKeys).Returns(["TestChatHistoryProvider"]); - mockChatHistoryProvider - .Protected() - .Setup>>("InvokingCoreAsync", ItExpr.IsAny(), ItExpr.IsAny()) - .Returns((ChatHistoryProvider.InvokingContext ctx, CancellationToken _) => - new ValueTask>(ctx.RequestMessages.ToList())); - mockChatHistoryProvider - .Protected() - .Setup("InvokedCoreAsync", ItExpr.IsAny(), ItExpr.IsAny()) - .Returns(() => new ValueTask()); - - ChatClientAgent agent = new(mockService.Object, options: new() - { - ChatHistoryProvider = mockChatHistoryProvider.Object, - PersistChatHistoryAtEndOfRun = false, - }); - - // Act - var inputMessage = new ChatMessage(ChatRole.User, "test"); - var session = await agent.CreateSessionAsync() as ChatClientAgentSession; - await agent.RunAsync([inputMessage], session); - - // Assert — input message should be marked as persisted - Assert.True( - inputMessage.AdditionalProperties?.ContainsKey(ChatHistoryPersistingChatClient.PersistedMarkerKey) == true, - "Input message should be marked as persisted after a successful run."); - } - /// - /// Verifies that when per-service-call persistence is enabled and the inner client returns a - /// conversation ID, the session's ConversationId is updated after the service call. + /// Verifies that when the inner client returns a real conversation ID, + /// the session's ConversationId is updated after the run. /// [Fact] - public async Task RunAsync_UpdatesSessionConversationId_WhenPerServiceCallPersistenceEnabledAsync() + public async Task RunAsync_UpdatesSessionConversationId_WhenServiceReturnsOneAsync() { // Arrange const string ExpectedConversationId = "conv-123"; @@ -741,10 +698,7 @@ public async Task RunAsync_UpdatesSessionConversationId_WhenPerServiceCallPersis ConversationId = ExpectedConversationId, }); - ChatClientAgent agent = new(mockService.Object, options: new() - { - PersistChatHistoryAtEndOfRun = false, - }); + ChatClientAgent agent = new(mockService.Object); // Act var session = await agent.CreateSessionAsync() as ChatClientAgentSession; @@ -766,8 +720,8 @@ private static async IAsyncEnumerable CreateAsyncEnumerableA /// /// Verifies that when per-service-call persistence is active and no real conversation ID exists, - /// sets the - /// sentinel on the chat options and strips it before + /// sets the + /// sentinel on the chat options and strips it before /// forwarding to the inner client. /// [Fact] @@ -787,7 +741,7 @@ public async Task RunAsync_SetsAndStripsSentinelConversationId_WhenPerServiceCal ChatClientAgent agent = new(mockService.Object, options: new() { ChatOptions = new() { Instructions = "test" }, - PersistChatHistoryAtEndOfRun = false, + SimulateServiceStoredChatHistory = true, }); // Act @@ -819,7 +773,7 @@ public async Task RunAsync_DoesNotSetSentinel_WhenEndOfRunPersistenceEnabledAsyn ChatClientAgent agent = new(mockService.Object, options: new() { ChatOptions = new() { Instructions = "test" }, - PersistChatHistoryAtEndOfRun = true, + SimulateServiceStoredChatHistory = true, }); // Act @@ -854,7 +808,7 @@ public async Task RunAsync_DoesNotSetSentinel_WhenRealConversationIdExistsAsync( ChatClientAgent agent = new(mockService.Object, options: new() { - PersistChatHistoryAtEndOfRun = false, + SimulateServiceStoredChatHistory = true, }); // Create a session with a real conversation ID. @@ -888,7 +842,7 @@ public async Task RunStreamingAsync_SetsAndStripsSentinelConversationId_WhenPerS ChatClientAgent agent = new(mockService.Object, options: new() { ChatOptions = new() { Instructions = "test" }, - PersistChatHistoryAtEndOfRun = false, + SimulateServiceStoredChatHistory = true, }); // Act @@ -903,11 +857,12 @@ public async Task RunStreamingAsync_SetsAndStripsSentinelConversationId_WhenPerS } /// - /// Verifies that the session's conversation ID is NOT set to the sentinel after the run. - /// The sentinel should only exist transiently on the ChatOptions for the pipeline. + /// Verifies that the session's conversation ID IS set to the sentinel after the run + /// when simulating service-stored chat history. This allows subsequent runs to + /// skip provider resolution in the agent (the decorator handles it). /// [Fact] - public async Task RunAsync_SentinelDoesNotLeakToSession_WhenPerServiceCallPersistenceActiveAsync() + public async Task RunAsync_SetsSentinelOnSession_WhenSimulateServiceStoredChatHistoryActiveAsync() { // Arrange Mock mockService = new(); @@ -920,14 +875,14 @@ public async Task RunAsync_SentinelDoesNotLeakToSession_WhenPerServiceCallPersis ChatClientAgent agent = new(mockService.Object, options: new() { - PersistChatHistoryAtEndOfRun = false, + SimulateServiceStoredChatHistory = true, }); // Act var session = await agent.CreateSessionAsync() as ChatClientAgentSession; await agent.RunAsync([new(ChatRole.User, "test")], session); - // Assert — session should NOT have the sentinel conversation ID - Assert.Null(session!.ConversationId); + // Assert — session should have the sentinel conversation ID + Assert.Equal(ServiceStoredSimulatingChatClient.LocalHistoryConversationId, session!.ConversationId); } } From 44e5ee8a822c7045b5bc5f91db51b49fd377b981 Mon Sep 17 00:00:00 2001 From: westey <164392973+westey-m@users.noreply.github.com> Date: Mon, 30 Mar 2026 14:40:29 +0000 Subject: [PATCH 2/6] Fixing bug in ServiceStoredSimulatingChatClient --- .../ServiceStoredSimulatingChatClient.cs | 93 ++--- .../ServiceStoredSimulatingChatClientTests.cs | 328 ++++++++++++++++++ 2 files changed, 380 insertions(+), 41 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI/ChatClient/ServiceStoredSimulatingChatClient.cs b/dotnet/src/Microsoft.Agents.AI/ChatClient/ServiceStoredSimulatingChatClient.cs index 710ffad9be..733adf6a6b 100644 --- a/dotnet/src/Microsoft.Agents.AI/ChatClient/ServiceStoredSimulatingChatClient.cs +++ b/dotnet/src/Microsoft.Agents.AI/ChatClient/ServiceStoredSimulatingChatClient.cs @@ -76,28 +76,38 @@ public override async Task GetResponseAsync( var (agent, session) = GetRequiredAgentAndSession(); options = StripLocalHistoryConversationId(options); - // Load history and prepend it to the messages. - var allMessages = await agent.LoadChatHistoryAsync(session, messages, options, cancellationToken).ConfigureAwait(false); + bool isServiceManaged = !string.IsNullOrEmpty(options?.ConversationId); + var newMessages = messages as IList ?? messages.ToList(); + + // When simulating, load history and prepend it. When the service manages + // history (real ConversationId), just forward the input messages as-is. + var messagesForService = isServiceManaged + ? newMessages + : await agent.LoadChatHistoryAsync(session, newMessages, options, cancellationToken).ConfigureAwait(false); ChatResponse response; try { - response = await base.GetResponseAsync(allMessages, options, cancellationToken).ConfigureAwait(false); + response = await base.GetResponseAsync(messagesForService, options, cancellationToken).ConfigureAwait(false); } catch (Exception ex) { - var newRequestMessages = GetNewRequestMessages(allMessages); - await agent.NotifyProvidersOfFailureAsync(session, ex, newRequestMessages, options, cancellationToken).ConfigureAwait(false); + await agent.NotifyProvidersOfFailureAsync(session, ex, newMessages, options, cancellationToken).ConfigureAwait(false); throw; } - var newMessages = GetNewRequestMessages(allMessages); - - // Persist immediately after each service call. await agent.NotifyProvidersOfNewMessagesAsync(session, newMessages, response.Messages, options, cancellationToken).ConfigureAwait(false); - // Set the sentinel ConversationId on the response and session. - SetSentinelConversationId(response, session); + // If the service returned (or already had) a real ConversationId, update the session + // with it. Otherwise set our sentinel so FICC treats this as service-managed. + if (isServiceManaged || !string.IsNullOrEmpty(response.ConversationId)) + { + agent.UpdateSessionConversationId(session, response.ConversationId, cancellationToken); + } + else + { + SetSentinelConversationId(response, session); + } return response; } @@ -111,20 +121,25 @@ public override async IAsyncEnumerable GetStreamingResponseA var (agent, session) = GetRequiredAgentAndSession(); options = StripLocalHistoryConversationId(options); - // Load history and prepend it to the messages. - var allMessages = await agent.LoadChatHistoryAsync(session, messages, options, cancellationToken).ConfigureAwait(false); + bool isServiceManaged = !string.IsNullOrEmpty(options?.ConversationId); + var newMessages = messages as IList ?? messages.ToList(); + + // When simulating, load history and prepend it. When the service manages + // history (real ConversationId), just forward the input messages as-is. + var messagesForService = isServiceManaged + ? newMessages + : await agent.LoadChatHistoryAsync(session, newMessages, options, cancellationToken).ConfigureAwait(false); List responseUpdates = []; IAsyncEnumerator enumerator; try { - enumerator = base.GetStreamingResponseAsync(allMessages, options, cancellationToken).GetAsyncEnumerator(cancellationToken); + enumerator = base.GetStreamingResponseAsync(messagesForService, options, cancellationToken).GetAsyncEnumerator(cancellationToken); } catch (Exception ex) { - var newRequestMessagesOnFailure = GetNewRequestMessages(allMessages); - await agent.NotifyProvidersOfFailureAsync(session, ex, newRequestMessagesOnFailure, options, cancellationToken).ConfigureAwait(false); + await agent.NotifyProvidersOfFailureAsync(session, ex, newMessages, options, cancellationToken).ConfigureAwait(false); throw; } @@ -135,8 +150,7 @@ public override async IAsyncEnumerable GetStreamingResponseA } catch (Exception ex) { - var newRequestMessagesOnFailure = GetNewRequestMessages(allMessages); - await agent.NotifyProvidersOfFailureAsync(session, ex, newRequestMessagesOnFailure, options, cancellationToken).ConfigureAwait(false); + await agent.NotifyProvidersOfFailureAsync(session, ex, newMessages, options, cancellationToken).ConfigureAwait(false); throw; } @@ -144,7 +158,18 @@ public override async IAsyncEnumerable GetStreamingResponseA { var update = enumerator.Current; responseUpdates.Add(update); - update.ConversationId = LocalHistoryConversationId; // Set the sentinel ConversationId on each update for the streaming case. + + // If the service returned a real ConversationId on any update, remember that. + // Otherwise stamp our sentinel so FICC treats this as service-managed. + if (!string.IsNullOrEmpty(update.ConversationId)) + { + isServiceManaged = true; + } + else if (!isServiceManaged) + { + update.ConversationId = LocalHistoryConversationId; + } + yield return update; try @@ -153,20 +178,23 @@ public override async IAsyncEnumerable GetStreamingResponseA } catch (Exception ex) { - var newRequestMessagesOnFailure = GetNewRequestMessages(allMessages); - await agent.NotifyProvidersOfFailureAsync(session, ex, newRequestMessagesOnFailure, options, cancellationToken).ConfigureAwait(false); + await agent.NotifyProvidersOfFailureAsync(session, ex, newMessages, options, cancellationToken).ConfigureAwait(false); throw; } } var chatResponse = responseUpdates.ToChatResponse(); - var newMessages = GetNewRequestMessages(allMessages); - // Persist immediately after each service call. await agent.NotifyProvidersOfNewMessagesAsync(session, newMessages, chatResponse.Messages, options, cancellationToken).ConfigureAwait(false); - // Set the sentinel ConversationId on the session. For streaming, set it on the last update. - session.ConversationId = LocalHistoryConversationId; + if (isServiceManaged) + { + agent.UpdateSessionConversationId(session, chatResponse.ConversationId, cancellationToken); + } + else + { + session.ConversationId = LocalHistoryConversationId; + } } /// @@ -204,23 +232,6 @@ private static (ChatClientAgent Agent, ChatClientAgentSession Session) GetRequir return (chatClientAgent, chatClientAgentSession); } - /// - /// Returns only the request messages that have not been loaded from chat history. - /// - /// - /// Messages loaded by the are tagged with - /// and should not be re-persisted. - /// Because treats the conversation as service-managed - /// (via the sentinel ), it clears accumulated history - /// between iterations, so only genuinely new messages arrive here. - /// - /// The full set of request messages to filter. - /// A list of request messages that need to be persisted. - private static List GetNewRequestMessages(IEnumerable messages) - { - return messages.Where(m => m.GetAgentRequestMessageSourceType() != AgentRequestMessageSourceType.ChatHistory).ToList(); - } - /// /// If the carry the sentinel, /// returns a clone with the conversation ID cleared so the inner client never sees it. diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ServiceStoredSimulatingChatClientTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ServiceStoredSimulatingChatClientTests.cs index 56329911c6..1648ceef92 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ServiceStoredSimulatingChatClientTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ServiceStoredSimulatingChatClientTests.cs @@ -885,4 +885,332 @@ public async Task RunAsync_SetsSentinelOnSession_WhenSimulateServiceStoredChatHi // Assert — session should have the sentinel conversation ID Assert.Equal(ServiceStoredSimulatingChatClient.LocalHistoryConversationId, session!.ConversationId); } + + /// + /// Verifies that when simulating service-stored chat history and the service returns + /// a real , the conflict detection in + /// throws because both a + /// and a service-managed ConversationId are present. + /// + [Fact] + public async Task RunAsync_Throws_WhenServiceReturnsRealConversationIdWithChatHistoryProviderAsync() + { + // Arrange + const string RealConversationId = "service-conv-456"; + + Mock mockChatHistoryProvider = new(null, null, null); + mockChatHistoryProvider.SetupGet(p => p.StateKeys).Returns(["TestChatHistoryProvider"]); + mockChatHistoryProvider + .Protected() + .Setup>>("InvokingCoreAsync", ItExpr.IsAny(), ItExpr.IsAny()) + .Returns((ChatHistoryProvider.InvokingContext ctx, CancellationToken _) => + new ValueTask>(ctx.RequestMessages.ToList())); + mockChatHistoryProvider + .Protected() + .Setup("InvokedCoreAsync", ItExpr.IsAny(), ItExpr.IsAny()) + .Returns(new ValueTask()); + + Mock mockService = new(); + mockService.Setup( + s => s.GetResponseAsync( + It.IsAny>(), + It.IsAny(), + It.IsAny())) + .ReturnsAsync(new ChatResponse([new(ChatRole.Assistant, "response")]) + { + ConversationId = RealConversationId, + }); + + ChatClientAgent agent = new(mockService.Object, options: new() + { + ChatHistoryProvider = mockChatHistoryProvider.Object, + SimulateServiceStoredChatHistory = true, + }); + + // Act & Assert — conflict detection should throw + var session = await agent.CreateSessionAsync() as ChatClientAgentSession; + await Assert.ThrowsAsync(() => agent.RunAsync([new(ChatRole.User, "test")], session)); + } + + /// + /// Verifies that when simulating service-stored chat history and the request carries a real + /// , the decorator skips history loading but still + /// notifies s on success and updates the session ConversationId. + /// + [Fact] + public async Task RunAsync_NotifiesProvidersAndUpdatesSession_WhenRequestHasRealConversationIdAsync() + { + // Arrange + const string RealConversationId = "real-conv-request"; + const string ServiceConversationId = "real-conv-response"; + + Mock mockContextProvider = new(null, null, null); + mockContextProvider.SetupGet(p => p.StateKeys).Returns(["TestContextProvider"]); + mockContextProvider + .Protected() + .Setup>("InvokingCoreAsync", ItExpr.IsAny(), ItExpr.IsAny()) + .Returns((AIContextProvider.InvokingContext ctx, CancellationToken _) => + new ValueTask(new AIContext { Messages = ctx.AIContext.Messages })); + mockContextProvider + .Protected() + .Setup("InvokedCoreAsync", ItExpr.IsAny(), ItExpr.IsAny()) + .Returns(new ValueTask()); + + Mock mockService = new(); + mockService.Setup( + s => s.GetResponseAsync( + It.IsAny>(), + It.IsAny(), + It.IsAny())) + .ReturnsAsync(new ChatResponse([new(ChatRole.Assistant, "response")]) + { + ConversationId = ServiceConversationId, + }); + + ChatClientAgent agent = new(mockService.Object, options: new() + { + SimulateServiceStoredChatHistory = true, + AIContextProviders = [mockContextProvider.Object], + }); + + // Create a session with a real conversation ID so it's on chatOptions. + var session = await agent.CreateSessionAsync(RealConversationId); + + // Act + await agent.RunAsync([new(ChatRole.User, "test")], session); + + // Assert — AIContextProvider.InvokedAsync should have been called + mockContextProvider + .Protected() + .Verify("InvokedCoreAsync", Times.Once(), + ItExpr.Is(x => + x.RequestMessages.Any(m => m.Text == "test") && + x.ResponseMessages!.Any(m => m.Text == "response")), + ItExpr.IsAny()); + + // Assert — session should have the service-returned ConversationId + Assert.Equal(ServiceConversationId, (session as ChatClientAgentSession)!.ConversationId); + } + + /// + /// Verifies that when simulating service-stored chat history and the request carries a real + /// , the decorator notifies providers of failure + /// when the inner client throws. + /// + [Fact] + public async Task RunAsync_NotifiesProvidersOfFailure_WhenRequestHasRealConversationIdAsync() + { + // Arrange + const string RealConversationId = "real-conv-failure"; + + Mock mockContextProvider = new(null, null, null); + mockContextProvider.SetupGet(p => p.StateKeys).Returns(["TestContextProvider"]); + mockContextProvider + .Protected() + .Setup>("InvokingCoreAsync", ItExpr.IsAny(), ItExpr.IsAny()) + .Returns((AIContextProvider.InvokingContext ctx, CancellationToken _) => + new ValueTask(new AIContext { Messages = ctx.AIContext.Messages })); + mockContextProvider + .Protected() + .Setup("InvokedCoreAsync", ItExpr.IsAny(), ItExpr.IsAny()) + .Returns(new ValueTask()); + + Mock mockService = new(); + mockService.Setup( + s => s.GetResponseAsync( + It.IsAny>(), + It.IsAny(), + It.IsAny())) + .ThrowsAsync(new InvalidOperationException("Service error")); + + ChatClientAgent agent = new(mockService.Object, options: new() + { + SimulateServiceStoredChatHistory = true, + AIContextProviders = [mockContextProvider.Object], + }); + + var session = await agent.CreateSessionAsync(RealConversationId); + + // Act & Assert — should throw + await Assert.ThrowsAsync(() => agent.RunAsync([new(ChatRole.User, "test")], session)); + + // Assert — AIContextProvider.InvokedAsync should have been called with the failure + mockContextProvider + .Protected() + .Verify("InvokedCoreAsync", Times.Once(), + ItExpr.Is(x => x.InvokeException != null), + ItExpr.IsAny()); + } + + /// + /// Verifies that in the streaming path, when the request carries a real + /// , the decorator skips history loading but still + /// notifies providers and updates the session ConversationId. + /// + [Fact] + public async Task RunStreamingAsync_NotifiesProvidersAndUpdatesSession_WhenRequestHasRealConversationIdAsync() + { + // Arrange + const string RealConversationId = "real-conv-streaming"; + const string ServiceConversationId = "service-conv-streaming"; + + Mock mockContextProvider = new(null, null, null); + mockContextProvider.SetupGet(p => p.StateKeys).Returns(["TestContextProvider"]); + mockContextProvider + .Protected() + .Setup>("InvokingCoreAsync", ItExpr.IsAny(), ItExpr.IsAny()) + .Returns((AIContextProvider.InvokingContext ctx, CancellationToken _) => + new ValueTask(new AIContext { Messages = ctx.AIContext.Messages })); + mockContextProvider + .Protected() + .Setup("InvokedCoreAsync", ItExpr.IsAny(), ItExpr.IsAny()) + .Returns(new ValueTask()); + + Mock mockService = new(); + mockService.Setup( + s => s.GetStreamingResponseAsync( + It.IsAny>(), + It.IsAny(), + It.IsAny())) + .Returns(CreateAsyncEnumerableAsync( + new ChatResponseUpdate(ChatRole.Assistant, "streamed") { ConversationId = ServiceConversationId })); + + ChatClientAgent agent = new(mockService.Object, options: new() + { + SimulateServiceStoredChatHistory = true, + AIContextProviders = [mockContextProvider.Object], + }); + + var session = await agent.CreateSessionAsync(RealConversationId); + + // Act + await foreach (var _ in agent.RunStreamingAsync([new(ChatRole.User, "test")], session)) + { + // Consume all updates. + } + + // Assert — AIContextProvider.InvokedAsync should have been called + mockContextProvider + .Protected() + .Verify("InvokedCoreAsync", Times.Once(), + ItExpr.IsAny(), + ItExpr.IsAny()); + + // Assert — session should have the service-returned ConversationId + Assert.Equal(ServiceConversationId, (session as ChatClientAgentSession)!.ConversationId); + } + + /// + /// Verifies that when simulating and the service unexpectedly returns a real + /// (no ConversationId on the request), the decorator + /// notifies providers and updates the session ConversationId without setting the sentinel. + /// + [Fact] + public async Task RunAsync_NotifiesProvidersAndUpdatesSession_WhenServiceReturnsUnexpectedConversationIdAsync() + { + // Arrange + const string ServiceConversationId = "unexpected-conv-id"; + + Mock mockContextProvider = new(null, null, null); + mockContextProvider.SetupGet(p => p.StateKeys).Returns(["TestContextProvider"]); + mockContextProvider + .Protected() + .Setup>("InvokingCoreAsync", ItExpr.IsAny(), ItExpr.IsAny()) + .Returns((AIContextProvider.InvokingContext ctx, CancellationToken _) => + new ValueTask(new AIContext { Messages = ctx.AIContext.Messages })); + mockContextProvider + .Protected() + .Setup("InvokedCoreAsync", ItExpr.IsAny(), ItExpr.IsAny()) + .Returns(new ValueTask()); + + Mock mockService = new(); + mockService.Setup( + s => s.GetResponseAsync( + It.IsAny>(), + It.IsAny(), + It.IsAny())) + .ReturnsAsync(new ChatResponse([new(ChatRole.Assistant, "response")]) + { + ConversationId = ServiceConversationId, + }); + + // No ChatHistoryProvider — so conflict detection won't throw. + ChatClientAgent agent = new(mockService.Object, options: new() + { + SimulateServiceStoredChatHistory = true, + AIContextProviders = [mockContextProvider.Object], + }); + + // Act + var session = await agent.CreateSessionAsync() as ChatClientAgentSession; + await agent.RunAsync([new(ChatRole.User, "test")], session); + + // Assert — AIContextProvider.InvokedAsync should have been called + mockContextProvider + .Protected() + .Verify("InvokedCoreAsync", Times.Once(), + ItExpr.Is(x => + x.ResponseMessages!.Any(m => m.Text == "response")), + ItExpr.IsAny()); + + // Assert — session should have the service ConversationId, not the sentinel + Assert.Equal(ServiceConversationId, session!.ConversationId); + } + + /// + /// Verifies that in the streaming path, when the service returns a real ConversationId mid-stream + /// (no ConversationId on the request), the decorator notifies providers and updates the session. + /// + [Fact] + public async Task RunStreamingAsync_NotifiesProvidersAndUpdatesSession_WhenServiceReturnsUnexpectedConversationIdAsync() + { + // Arrange + const string ServiceConversationId = "unexpected-stream-conv"; + + Mock mockContextProvider = new(null, null, null); + mockContextProvider.SetupGet(p => p.StateKeys).Returns(["TestContextProvider"]); + mockContextProvider + .Protected() + .Setup>("InvokingCoreAsync", ItExpr.IsAny(), ItExpr.IsAny()) + .Returns((AIContextProvider.InvokingContext ctx, CancellationToken _) => + new ValueTask(new AIContext { Messages = ctx.AIContext.Messages })); + mockContextProvider + .Protected() + .Setup("InvokedCoreAsync", ItExpr.IsAny(), ItExpr.IsAny()) + .Returns(new ValueTask()); + + Mock mockService = new(); + mockService.Setup( + s => s.GetStreamingResponseAsync( + It.IsAny>(), + It.IsAny(), + It.IsAny())) + .Returns(CreateAsyncEnumerableAsync( + new ChatResponseUpdate(ChatRole.Assistant, "part1"), + new ChatResponseUpdate(null, "part2") { ConversationId = ServiceConversationId })); + + // No ChatHistoryProvider — so conflict detection won't throw. + ChatClientAgent agent = new(mockService.Object, options: new() + { + SimulateServiceStoredChatHistory = true, + AIContextProviders = [mockContextProvider.Object], + }); + + // Act + var session = await agent.CreateSessionAsync() as ChatClientAgentSession; + await foreach (var _ in agent.RunStreamingAsync([new(ChatRole.User, "test")], session)) + { + // Consume all updates. + } + + // Assert — AIContextProvider.InvokedAsync should have been called + mockContextProvider + .Protected() + .Verify("InvokedCoreAsync", Times.Once(), + ItExpr.IsAny(), + ItExpr.IsAny()); + + // Assert — session should have the service ConversationId, not the sentinel + Assert.Equal(ServiceConversationId, session!.ConversationId); + } } From 50c2cad170976d8052a6fd3e9b04790d562b9e52 Mon Sep 17 00:00:00 2001 From: westey <164392973+westey-m@users.noreply.github.com> Date: Mon, 30 Mar 2026 14:45:55 +0000 Subject: [PATCH 3/6] Addressing PR comments. --- .../ChatClient/ChatClientAgentLogMessages.cs | 2 +- .../ChatClient/ChatClientAgentOptions.cs | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgentLogMessages.cs b/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgentLogMessages.cs index 9d6977e716..5899ef5cb2 100644 --- a/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgentLogMessages.cs +++ b/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgentLogMessages.cs @@ -92,7 +92,7 @@ public static partial void LogAgentChatClientMissingPersistingClient( /// [LoggerMessage( Level = LogLevel.Warning, - Message = "Agent {AgentId}/{AgentName}: SimulateServiceStoredChatHistory is enabled but we have to fall back to end-of-run persistence because the run involves background responses. Messages will be marked during the run and persisted at the end.")] + Message = "Agent {AgentId}/{AgentName}: SimulateServiceStoredChatHistory is enabled but we have to fall back to end-of-run persistence because the run involves background responses.")] public static partial void LogAgentChatClientBackgroundResponseFallback( this ILogger logger, string agentId, diff --git a/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgentOptions.cs b/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgentOptions.cs index 4387d8c5ba..e3d4326ae6 100644 --- a/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgentOptions.cs +++ b/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgentOptions.cs @@ -117,7 +117,11 @@ public sealed class ChatClientAgentOptions /// pipeline. /// /// - /// This option has no effect when is . + /// When setting the setting to and + /// to , ensure that your custom chat client stack includes a + /// to enable per-service-call persistence. + /// If no is provided, and you are not storing chat history via other means, + /// no chat history may be stored. /// When using a custom chat client stack, you can add a /// manually via the /// extension method. From f9c43f916e0b9ae9008a55173ec64ee2fd2418ac Mon Sep 17 00:00:00 2001 From: westey <164392973+westey-m@users.noreply.github.com> Date: Mon, 30 Mar 2026 15:11:43 +0000 Subject: [PATCH 4/6] Address PR comments --- .../ServiceStoredSimulatingChatClient.cs | 43 ++++++-- .../ServiceStoredSimulatingChatClientTests.cs | 98 +++++++++++++++++++ 2 files changed, 131 insertions(+), 10 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI/ChatClient/ServiceStoredSimulatingChatClient.cs b/dotnet/src/Microsoft.Agents.AI/ChatClient/ServiceStoredSimulatingChatClient.cs index 733adf6a6b..f7dafa303d 100644 --- a/dotnet/src/Microsoft.Agents.AI/ChatClient/ServiceStoredSimulatingChatClient.cs +++ b/dotnet/src/Microsoft.Agents.AI/ChatClient/ServiceStoredSimulatingChatClient.cs @@ -77,11 +77,16 @@ public override async Task GetResponseAsync( options = StripLocalHistoryConversationId(options); bool isServiceManaged = !string.IsNullOrEmpty(options?.ConversationId); + bool isContinuationOrBackground = options?.ContinuationToken is not null + || options?.AllowBackgroundResponses is true; + bool skipSimulation = isServiceManaged || isContinuationOrBackground; + var newMessages = messages as IList ?? messages.ToList(); // When simulating, load history and prepend it. When the service manages - // history (real ConversationId), just forward the input messages as-is. - var messagesForService = isServiceManaged + // history (real ConversationId) or this is a continuation/background run, + // just forward the input messages as-is. + var messagesForService = skipSimulation ? newMessages : await agent.LoadChatHistoryAsync(session, newMessages, options, cancellationToken).ConfigureAwait(false); @@ -98,14 +103,19 @@ public override async Task GetResponseAsync( await agent.NotifyProvidersOfNewMessagesAsync(session, newMessages, response.Messages, options, cancellationToken).ConfigureAwait(false); - // If the service returned (or already had) a real ConversationId, update the session - // with it. Otherwise set our sentinel so FICC treats this as service-managed. - if (isServiceManaged || !string.IsNullOrEmpty(response.ConversationId)) + if (isContinuationOrBackground) + { + // Continuation/background run — the agent's forced end-of-run handles + // session ConversationId and persistence; the decorator is a no-op. + } + else if (isServiceManaged || !string.IsNullOrEmpty(response.ConversationId)) { + // Service manages history — update session with the real ConversationId. agent.UpdateSessionConversationId(session, response.ConversationId, cancellationToken); } else { + // Normal simulated path — set sentinel so FICC treats this as service-managed. SetSentinelConversationId(response, session); } @@ -122,11 +132,16 @@ public override async IAsyncEnumerable GetStreamingResponseA options = StripLocalHistoryConversationId(options); bool isServiceManaged = !string.IsNullOrEmpty(options?.ConversationId); + bool isContinuationOrBackground = options?.ContinuationToken is not null + || options?.AllowBackgroundResponses is true; + bool skipSimulation = isServiceManaged || isContinuationOrBackground; + var newMessages = messages as IList ?? messages.ToList(); // When simulating, load history and prepend it. When the service manages - // history (real ConversationId), just forward the input messages as-is. - var messagesForService = isServiceManaged + // history (real ConversationId) or this is a continuation/background run, + // just forward the input messages as-is. + var messagesForService = skipSimulation ? newMessages : await agent.LoadChatHistoryAsync(session, newMessages, options, cancellationToken).ConfigureAwait(false); @@ -160,12 +175,13 @@ public override async IAsyncEnumerable GetStreamingResponseA responseUpdates.Add(update); // If the service returned a real ConversationId on any update, remember that. - // Otherwise stamp our sentinel so FICC treats this as service-managed. + // Otherwise stamp our sentinel so FICC treats this as service-managed — + // unless this is a continuation/background run where the agent handles everything. if (!string.IsNullOrEmpty(update.ConversationId)) { isServiceManaged = true; } - else if (!isServiceManaged) + else if (!skipSimulation) { update.ConversationId = LocalHistoryConversationId; } @@ -187,12 +203,19 @@ public override async IAsyncEnumerable GetStreamingResponseA await agent.NotifyProvidersOfNewMessagesAsync(session, newMessages, chatResponse.Messages, options, cancellationToken).ConfigureAwait(false); - if (isServiceManaged) + if (isContinuationOrBackground) + { + // Continuation/background run — the agent's forced end-of-run handles + // session ConversationId and persistence; the decorator is a no-op. + } + else if (isServiceManaged) { + // Service manages history — update session with the real ConversationId. agent.UpdateSessionConversationId(session, chatResponse.ConversationId, cancellationToken); } else { + // Normal simulated path — set sentinel on session. session.ConversationId = LocalHistoryConversationId; } } diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ServiceStoredSimulatingChatClientTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ServiceStoredSimulatingChatClientTests.cs index 1648ceef92..ea346dade9 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ServiceStoredSimulatingChatClientTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ServiceStoredSimulatingChatClientTests.cs @@ -1213,4 +1213,102 @@ public async Task RunStreamingAsync_NotifiesProvidersAndUpdatesSession_WhenServi // Assert — session should have the service ConversationId, not the sentinel Assert.Equal(ServiceConversationId, session!.ConversationId); } + + /// + /// Verifies that when is true, + /// the decorator skips history loading and sentinel setting, letting the agent's + /// forced end-of-run path handle persistence. + /// + [Fact] + public async Task RunAsync_SkipsSimulation_WhenAllowBackgroundResponsesAsync() + { + // Arrange + IEnumerable? capturedMessages = null; + Mock mockService = new(); + mockService.Setup( + s => s.GetResponseAsync( + It.IsAny>(), + It.IsAny(), + It.IsAny())) + .Callback, ChatOptions?, CancellationToken>((msgs, _, _) => capturedMessages = msgs) + .ReturnsAsync(new ChatResponse([new(ChatRole.Assistant, "response")])); + + Mock mockChatHistoryProvider = new(null, null, null); + mockChatHistoryProvider.SetupGet(p => p.StateKeys).Returns(["TestChatHistoryProvider"]); + mockChatHistoryProvider + .Protected() + .Setup>>("InvokingCoreAsync", ItExpr.IsAny(), ItExpr.IsAny()) + .Returns((ChatHistoryProvider.InvokingContext ctx, CancellationToken _) => + { + // Add a history message to verify it's NOT prepended in this scenario. + var result = ctx.RequestMessages.ToList(); + result.Insert(0, new ChatMessage(ChatRole.Assistant, "history")); + return new ValueTask>(result); + }); + mockChatHistoryProvider + .Protected() + .Setup("InvokedCoreAsync", ItExpr.IsAny(), ItExpr.IsAny()) + .Returns(new ValueTask()); + + ChatClientAgent agent = new(mockService.Object, options: new() + { + ChatHistoryProvider = mockChatHistoryProvider.Object, + SimulateServiceStoredChatHistory = true, + }); + + // Act + var session = await agent.CreateSessionAsync() as ChatClientAgentSession; + await agent.RunAsync( + [new(ChatRole.User, "test")], + session, + new AgentRunOptions { AllowBackgroundResponses = true }); + + // Assert — the inner client should NOT have received history messages + Assert.NotNull(capturedMessages); + var messageList = capturedMessages!.ToList(); + Assert.Single(messageList); + Assert.Equal("test", messageList[0].Text); + + // Assert — session should NOT have the sentinel (agent handles ConversationId at end-of-run) + Assert.NotEqual(ServiceStoredSimulatingChatClient.LocalHistoryConversationId, session!.ConversationId); + } + + /// + /// Verifies that in the streaming path, when is true, + /// the decorator skips history loading and sentinel setting. + /// + [Fact] + public async Task RunStreamingAsync_SkipsSimulation_WhenAllowBackgroundResponsesAsync() + { + // Arrange + Mock mockService = new(); + mockService.Setup( + s => s.GetStreamingResponseAsync( + It.IsAny>(), + It.IsAny(), + It.IsAny())) + .Returns(CreateAsyncEnumerableAsync(new ChatResponseUpdate(ChatRole.Assistant, "response"))); + + ChatClientAgent agent = new(mockService.Object, options: new() + { + SimulateServiceStoredChatHistory = true, + }); + + // Act + var session = await agent.CreateSessionAsync() as ChatClientAgentSession; + List updates = []; + await foreach (var update in agent.RunStreamingAsync( + [new(ChatRole.User, "test")], + session, + new AgentRunOptions { AllowBackgroundResponses = true })) + { + updates.Add(update); + } + + // Assert — updates should NOT carry the sentinel ConversationId + Assert.NotEmpty(updates); + + // Assert — session should NOT have the sentinel + Assert.NotEqual(ServiceStoredSimulatingChatClient.LocalHistoryConversationId, session!.ConversationId); + } } From ae326f790f0ec1b4097b363426bf4ba04b35a1fe Mon Sep 17 00:00:00 2001 From: westey <164392973+westey-m@users.noreply.github.com> Date: Mon, 30 Mar 2026 17:34:23 +0100 Subject: [PATCH 5/6] Apply suggestion from @SergeyMenshykh Co-authored-by: SergeyMenshykh <68852919+SergeyMenshykh@users.noreply.github.com> --- docs/decisions/0022-chat-history-persistence-consistency.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/decisions/0022-chat-history-persistence-consistency.md b/docs/decisions/0022-chat-history-persistence-consistency.md index 08a31bc88a..2784934817 100644 --- a/docs/decisions/0022-chat-history-persistence-consistency.md +++ b/docs/decisions/0022-chat-history-persistence-consistency.md @@ -41,7 +41,7 @@ The persistence timing and `FunctionResultContent` trimming behaviors are interr ## Considered Options -- Option 1: Per-run persistence with opt-in FRC trimming +- Option 1: Per-run persistence with opt-in FRC (FunctionResultContent) trimming - Option 2: Opt-in per-service-call persistence (via `SimulateServiceStoredChatHistory`) ## Pros and Cons of the Options From 972520be1043cf62c9253cf7149a2c592af3b803 Mon Sep 17 00:00:00 2001 From: westey <164392973+westey-m@users.noreply.github.com> Date: Mon, 30 Mar 2026 19:06:32 +0100 Subject: [PATCH 6/6] Fix bug --- .../ChatClient/ChatClientAgent.cs | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgent.cs b/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgent.cs index 49f4324397..3e0a2a7ed6 100644 --- a/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgent.cs +++ b/dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgent.cs @@ -719,6 +719,24 @@ private async Task throw new InvalidOperationException("Input messages are not allowed when continuing a background response using a continuation token."); } + // If a user provided two different session ids, via the session object and options, we should throw + // since we don't know which one to use. + if (!string.IsNullOrWhiteSpace(typedSession.ConversationId) && !string.IsNullOrWhiteSpace(chatOptions?.ConversationId) && typedSession.ConversationId != chatOptions!.ConversationId) + { + throw new InvalidOperationException( + $""" + The {nameof(chatOptions.ConversationId)} provided via {nameof(this.ChatOptions)} is different to the id of the provided {nameof(AgentSession)}. + Only one id can be used for a run. + """); + } + + // Only create or update ChatOptions if we have an id on the session and we don't have the same one already in ChatOptions. + if (!string.IsNullOrWhiteSpace(typedSession.ConversationId) && typedSession.ConversationId != chatOptions?.ConversationId) + { + chatOptions ??= new(); + chatOptions.ConversationId = typedSession.ConversationId; + } + IEnumerable inputMessagesForChatClient = inputMessages; // Populate the session messages only if we are not continuing an existing response as it's not allowed. @@ -767,24 +785,6 @@ private async Task } } - // If a user provided two different session ids, via the session object and options, we should throw - // since we don't know which one to use. - if (!string.IsNullOrWhiteSpace(typedSession.ConversationId) && !string.IsNullOrWhiteSpace(chatOptions?.ConversationId) && typedSession.ConversationId != chatOptions!.ConversationId) - { - throw new InvalidOperationException( - $""" - The {nameof(chatOptions.ConversationId)} provided via {nameof(this.ChatOptions)} is different to the id of the provided {nameof(AgentSession)}. - Only one id can be used for a run. - """); - } - - // Only create or update ChatOptions if we have an id on the session and we don't have the same one already in ChatOptions. - if (!string.IsNullOrWhiteSpace(typedSession.ConversationId) && typedSession.ConversationId != chatOptions?.ConversationId) - { - chatOptions ??= new(); - chatOptions.ConversationId = typedSession.ConversationId; - } - // Materialize the accumulated messages once at the end of the provider pipeline, reusing the existing list if possible. List messagesList = inputMessagesForChatClient as List ?? inputMessagesForChatClient.ToList();