Skip to content

Optimize auth record export.#1267

Merged
bwateratmsft merged 6 commits intomicrosoft:mainfrom
g2vinay:optimize-auth-record-export
Oct 30, 2025
Merged

Optimize auth record export.#1267
bwateratmsft merged 6 commits intomicrosoft:mainfrom
g2vinay:optimize-auth-record-export

Conversation

@g2vinay
Copy link
Copy Markdown
Contributor

@g2vinay g2vinay commented Oct 14, 2025

This PR introduces account state management to optimize authentication record exports by only processing meaningful account changes (additions/removals) while maintaining thread-safety.

Benefits

  • 70-90% reduction in session fetch operations
  • 98% faster for non-change scenarios (token refresh, session updates)
  • No regression for new account scenarios
  • Thread-safe concurrent request handling eliminates duplicate API calls

@g2vinay g2vinay requested a review from a team as a code owner October 14, 2025 23:45
alexweininger
alexweininger previously approved these changes Oct 15, 2025
Copy link
Copy Markdown
Member

@scottaddie scottaddie left a comment

Choose a reason for hiding this comment

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

Has this change been tested in both the client app and the /azure web experience?

Comment thread src/exportAuthRecord.ts Outdated
Comment thread src/exportAuthRecord.ts
Comment thread src/exportAuthRecord.ts
@g2vinay
Copy link
Copy Markdown
Contributor Author

g2vinay commented Oct 17, 2025

Has this change been tested in both the client app and the /azure web experience?

client app yes.
For VS Code Web, I don't see any way to supply the local built .vsix file.
I am looking around to see, if there's any way for us to test for web.
@bwateratmsft , do you know of any method we can use ?

@alexweininger
Copy link
Copy Markdown
Member

@g2vinay testing on /azure I didn't see the ~/.azure/authRecord.json file. But I didn't see it before I installed it either. Is it not working currently on /azure?

Comment thread src/exportAuthRecord.ts Outdated
@bwateratmsft
Copy link
Copy Markdown
Contributor

@g2vinay testing on /azure I didn't see the ~/.azure/authRecord.json file. But I didn't see it before I installed it either. Is it not working currently on /azure?

It worked for me, did Resources actually activate?

@alexweininger
Copy link
Copy Markdown
Member

@g2vinay testing on /azure I didn't see the ~/.azure/authRecord.json file. But I didn't see it before I installed it either. Is it not working currently on /azure?

It worked for me, did Resources actually activate?

I made sure to load the tree view. I'll try again

@bwateratmsft
Copy link
Copy Markdown
Contributor

@g2vinay testing on /azure I didn't see the ~/.azure/authRecord.json file. But I didn't see it before I installed it either. Is it not working currently on /azure?

It worked for me, did Resources actually activate?

I made sure to load the tree view. I'll try again

Oh BTW the path is in ~/.azure/ms-azuretools.vscode-azureresourcegroups.

@alexweininger
Copy link
Copy Markdown
Member

@g2vinay testing on /azure I didn't see the ~/.azure/authRecord.json file. But I didn't see it before I installed it either. Is it not working currently on /azure?

It worked for me, did Resources actually activate?

I made sure to load the tree view. I'll try again

Oh BTW the path is in ~/.azure/ms-azuretools.vscode-azureresourcegroups.

Yup, found it now. It's working as intented

alexweininger
alexweininger previously approved these changes Oct 20, 2025
Copy link
Copy Markdown
Contributor

@bwateratmsft bwateratmsft left a comment

Choose a reason for hiding this comment

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

Ok, unfortunately it is still happening in /azure, just more quietly. Calling vscode.authentication.getAccounts() is enough to trigger vscode.authentication.onDidChangeSessions, so we need to work something else out.

@g2vinay
Copy link
Copy Markdown
Contributor Author

g2vinay commented Oct 22, 2025

Ok, unfortunately it is still happening in /azure, just more quietly. Calling vscode.authentication.getAccounts() is enough to trigger vscode.authentication.onDidChangeSessions, so we need to work something else out.

We can consider, disabling this for VS code web as an alternative option.
Since WAM isn't available in VS Code Web, this feature doesn't offer any value in this scenario.

@alexweininger
Copy link
Copy Markdown
Member

We can consider, disabling this for VS code web as an alternative option.
Since WAM isn't available in VS Code Web, this feature doesn't offer any value in this scenario.

Sounds fine to me

@bwateratmsft
Copy link
Copy Markdown
Contributor

Sounds good, let's plan on doing that for now. When we reenable it, we should address #1226 too. @alexweininger what's the best way to detect if it's /azure?

@alexweininger
Copy link
Copy Markdown
Member

Sounds good, let's plan on doing that for now. When we reenable it, we should address #1226 too. @alexweininger what's the best way to detect if it's /azure?

Cloud Shell folks have instructed me to look for the AZD_IN_CLOUDSHELL=1 environment variable as a reliable way to detect if you're currently running in Cloud Shell.

@bwateratmsft bwateratmsft requested a review from Copilot October 27, 2025 19:05
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 introduces AuthAccountStateManager to optimize authentication record exports by tracking account state changes and preventing unnecessary API calls. The optimization focuses on detecting meaningful changes (account additions/removals) to avoid processing token refreshes and session updates.

Key Changes:

  • Introduced singleton AuthAccountStateManager class with thread-safe concurrent request handling
  • Modified exportAuthRecord to use state manager and only process meaningful account changes
  • Updated getAuthenticationSession to use cached accounts instead of fetching fresh data

Reviewed Changes

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

File Description
src/exportAuthRecord.ts Implements AuthAccountStateManager singleton class and refactors exportAuthRecord to use change detection logic
test/authAccountStateManager.test.ts Adds comprehensive test suite for AuthAccountStateManager functionality
extension.bundle.ts Exports AuthAccountStateManager and getAuthAccountStateManager for testing purposes

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/exportAuthRecord.ts Outdated
Comment thread test/authAccountStateManager.test.ts
bwateratmsft and others added 2 commits October 30, 2025 12:17
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@bwateratmsft bwateratmsft enabled auto-merge (squash) October 30, 2025 16:22
@bwateratmsft bwateratmsft merged commit f7a63f3 into microsoft:main Oct 30, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants