Skip to content

Conversation

@SergeyMenshykh
Copy link
Member

Motivation and Context

This PR updates CosmosDB-related extension methods, allowing them to accept authentication token credentials instead of using a default one, which may not be deterministic enough for production scenarios.

@SergeyMenshykh SergeyMenshykh self-assigned this Jan 16, 2026
Copilot AI review requested due to automatic review settings January 16, 2026 13:44
@SergeyMenshykh SergeyMenshykh added .NET agents Issues related to single agents labels Jan 16, 2026
@SergeyMenshykh SergeyMenshykh moved this to In Progress in Agent Framework Jan 16, 2026
Copy link
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 updates CosmosDB extension methods to accept TokenCredential parameters for authentication instead of hardcoding DefaultAzureCredential, providing more flexibility for production scenarios. This is a breaking change as it modifies the method signatures.

Changes:

  • Replaced Azure.Identity namespace import with Azure.Core in both extension files
  • Added TokenCredential parameter to CreateCheckpointStoreUsingManagedIdentity methods (both generic and non-generic)
  • Added TokenCredential parameter to WithCosmosDBMessageStoreUsingManagedIdentity method
  • Added null validation for the new tokenCredential parameter in all modified methods

Reviewed changes

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

File Description
CosmosDBWorkflowExtensions.cs Added tokenCredential parameter to both CreateCheckpointStoreUsingManagedIdentity overloads with validation
CosmosDBChatExtensions.cs Added tokenCredential parameter to WithCosmosDBMessageStoreUsingManagedIdentity with validation

Copy link
Member

@lokitoth lokitoth left a comment

Choose a reason for hiding this comment

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

We should also consider letting the user provide a CosmosClient directly; I imagine people building webapps will store more data than just the chat history, and it is likely that they will reuse a CosmosDB account for it, which will likely be configured on DI.

@westey-m
Copy link
Contributor

We should also consider letting the user provide a CosmosClient directly

We actually have a work item (#2982) for this already and @TheovanKraay, who contributed these implementations, said that he can take a look.

@TheovanKraay
Copy link
Contributor

I agree with supporting explicit TokenCredential, but I don't think this needs to be breaking, and would would not want to remove implicit DefaultAzureCredential usage anyway, which this change does. Can it be refactored so existing methods remain and forward to new overloads that accept TokenCredential?

@TheovanKraay
Copy link
Contributor

We should also consider letting the user provide a CosmosClient directly

We actually have a work item (#2982) for this already and @TheovanKraay, who contributed these implementations, said that he can take a look.

I will look at this next week. I agree that is also needed for more complex scenarios than this.

SergeyMenshykh and others added 3 commits January 16, 2026 16:28
…ensions.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ensions.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ons.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@TheovanKraay
Copy link
Contributor

I agree with supporting explicit TokenCredential, but I don't think this needs to be breaking, and would would not want to remove implicit DefaultAzureCredential usage anyway, which this change does. Can it be refactored so existing methods remain and forward to new overloads that accept TokenCredential?

As discussed, the change does actually make sense, even as a breaking change. Fine for it to be merged.

@SergeyMenshykh SergeyMenshykh added this pull request to the merge queue Jan 22, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 22, 2026
@SergeyMenshykh SergeyMenshykh added this pull request to the merge queue Jan 22, 2026
Merged via the queue into microsoft:main with commit f47645c Jan 22, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Issues related to single agents .NET

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants