Skip to content

.NET: Fix bug with per-service-call persistence and approvals#4933

Merged
westey-m merged 2 commits intomicrosoft:mainfrom
westey-m:per-service-call-approvals-fix
Mar 26, 2026
Merged

.NET: Fix bug with per-service-call persistence and approvals#4933
westey-m merged 2 commits intomicrosoft:mainfrom
westey-m:per-service-call-approvals-fix

Conversation

@westey-m
Copy link
Copy Markdown
Contributor

Motivation and Context

#2889
#4791

Description

  • Fix bug with per-service-call persistence and approvals
  • Add more tests to validate

Contribution Checklist

  • The code builds clean without any errors or warnings
  • The PR follows the Contribution Guidelines
  • All unit tests pass, and I have added new tests where possible
  • Is this a breaking change? If yes, add "[BREAKING]" prefix to the title of the PR.

Copilot AI review requested due to automatic review settings March 26, 2026 16:34
@github-actions github-actions bot changed the title Fix bug with per-service-call persistence and approvals .NET: Fix bug with per-service-call persistence and approvals Mar 26, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses a bug in .NET ChatClientAgent approval flows when per-service-call persistence is enabled by introducing a local-history sentinel ConversationId to influence FunctionInvokingChatClient behavior (while stripping it before reaching the leaf client), and adds end-to-end unit tests to validate persistence + approvals behavior.

Changes:

  • Add a sentinel ChatOptions.ConversationId (LocalHistoryConversationId) for per-service-call persistence, and strip it before calling the leaf IChatClient.
  • Expand unit tests to validate sentinel behavior (non-leakage), chat options merging expectations, persistence behavior, and approval flows.
  • Introduce a shared test helper to run multi-call scenarios with history assertions.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgent.cs Sets a local-history sentinel ConversationId for per-service-call persistence runs.
dotnet/src/Microsoft.Agents.AI/ChatClient/ChatHistoryPersistingChatClient.cs Adds sentinel constant and strips it before forwarding options to the inner client.
dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatHistoryPersistingChatClientTests.cs Adds tests ensuring sentinel is stripped and does not leak to session/inner client.
dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgent_ChatOptionsMergingTests.cs Updates expectations around null vs non-null ChatOptions due to sentinel behavior.
dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgentTests.cs Updates tests to capture options and assert ConversationId is stripped.
dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgent_ChatHistoryManagementTests.cs Adds end-to-end persistence validation tests across modes and function loops.
dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgent_ApprovalsTests.cs New end-to-end approval flow tests across persistence modes and rejection scenarios.
dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgentTestHelper.cs New helper for sequential mock service calls + persisted history assertions.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants