Fix tree refresh regressions (#1412, #1413)#1415
Conversation
Fix #1412: Restore explicit cache-clear and tree refresh after updating subscription selection. The config-change event alone does not clear the subscription cache, so re-selecting a subscription rendered stale data. Fix #1413: Convert the one-shot cache-clear boolean to a counter so multiple trees can each consume a cache-clear token independently. Previously only the first tree to load (Azure Resources) received noCache: true; the Accounts & Tenants tree loaded from stale cache and missed newly added accounts. Also fix the same one-shot pattern in signInToTenant and configureSovereignCloud commands. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes recent tree refresh regressions introduced by #1383 by restoring explicit refresh behavior and improving cache invalidation so multiple views can reliably reload fresh data after auth/subscription changes.
Changes:
- Reintroduced explicit Azure Resources refresh (with cache-clear) after subscription selection updates.
- Replaced one-shot cache-clear boolean with a counter so multiple trees can each consume a cache-clear signal.
- Updated sign-in / sovereign cloud / tenant sign-in command flows to request cache clears per refreshed tree.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tree/tenants/TenantResourceTreeDataProvider.ts | Updates comment to reflect token-based cache-clear consumption on tenant tree load. |
| src/tree/azure/FocusViewTreeDataProvider.ts | Updates comment to reflect token-based cache-clear consumption on focus view load. |
| src/tree/azure/AzureResourceTreeDataProvider.ts | Updates comment to reflect token-based cache-clear consumption on Azure Resources tree load. |
| src/extensionVariables.ts | Converts cache-clear mechanism from boolean flag to consumable counter. |
| src/commands/sovereignCloud/configureSovereignCloud.ts | Requests cache clear separately for Azure and Tenant tree refreshes. |
| src/commands/registerCommands.ts | Requests cache clear separately for Tenant and Azure refresh after signInToTenant commands. |
| src/commands/accounts/selectSubscriptions.ts | Restores explicit Azure tree refresh with cache clear after updating selected subscriptions. |
| src/commands/accounts/logIn.ts | Requests cache clear separately for Azure and Tenant refresh after sign-in completes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * Cache invalidation flag. When set to true, the next call to `consumeClearCacheFlag()` | ||
| * will return true and atomically reset the flag to false. This prevents race conditions | ||
| * where multiple trees might read and reset the flag independently. | ||
| * Cache invalidation counter. Each call to `setClearCacheOnNextLoad()` increments | ||
| * the counter by one. Each call to `consumeClearCacheFlag()` decrements it (if > 0) | ||
| * and returns true. This allows multiple trees to each consume a cache-clear signal | ||
| * independently (e.g. after sign-in, both the Azure and Tenant trees need fresh data). | ||
| */ | ||
| let clearCacheOnNextLoadFlag: boolean = false; | ||
| let clearCacheOnNextLoadCount: number = 0; | ||
|
|
||
| /** | ||
| * Sets the flag to clear auth caches on the next load. | ||
| * Requests a cache clear on the next tree load. Call once per tree that should | ||
| * receive a cache-clear signal. | ||
| */ | ||
| export function setClearCacheOnNextLoad(): void { | ||
| clearCacheOnNextLoadFlag = true; | ||
| clearCacheOnNextLoadCount++; | ||
| } |
There was a problem hiding this comment.
clearCacheOnNextLoadCount can now grow without bound if setClearCacheOnNextLoad() is called when no consumer ends up calling consumeClearCacheFlag() (e.g., a tree refresh that returns early before reaching the consume call). This can lead to unexpected repeated cache clears on future loads. Consider capping the counter to the number of consuming trees, or switching to a per-tree/per-reason mechanism (e.g., a map/bitmask) so tokens can’t accumulate indefinitely or be consumed by the wrong tree.
See below for a potential fix:
const MAX_CLEAR_CACHE_TOKENS: number = 8;
let clearCacheOnNextLoadCount: number = 0;
/**
* Requests a cache clear on the next tree load. Call once per tree that should
* receive a cache-clear signal.
*
* The number of outstanding tokens is capped to avoid unbounded growth if
* `setClearCacheOnNextLoad()` is called without a corresponding
* `consumeClearCacheFlag()` (for example, if a refresh returns early).
*/
export function setClearCacheOnNextLoad(): void {
if (clearCacheOnNextLoadCount < MAX_CLEAR_CACHE_TOKENS) {
clearCacheOnNextLoadCount++;
}
| /** | ||
| * Atomically consumes the clear cache flag. Returns true if caches should be cleared, | ||
| * and resets the flag to false. This ensures only the first consumer gets `true`. | ||
| * Atomically consumes one cache-clear token. Returns true if caches should be | ||
| * cleared, and decrements the counter so other trees can still consume theirs. | ||
| */ | ||
| export function consumeClearCacheFlag(): boolean { | ||
| const shouldClear = clearCacheOnNextLoadFlag; | ||
| clearCacheOnNextLoadFlag = false; | ||
| return shouldClear; | ||
| if (clearCacheOnNextLoadCount > 0) { | ||
| clearCacheOnNextLoadCount--; | ||
| return true; | ||
| } | ||
| return false; | ||
| } |
There was a problem hiding this comment.
consumeClearCacheFlag() no longer represents a boolean "flag"—it now consumes a token from a counter. Since it’s only used internally, consider renaming it (and the related variable) to reflect the new semantics (e.g., consumeClearCacheToken / clearCacheOnNextLoadTokenCount) to avoid future misuse or incorrect assumptions.
| await setSelectedTenantAndSubscriptionIds(Array.from(previouslySelectedSubscriptionsSettingValue)); | ||
|
|
||
| // Explicitly refresh the Azure tree with a cache clear so the view | ||
| // reflects the updated subscription selection immediately. | ||
| ext.setClearCacheOnNextLoad(); | ||
| ext.actions.refreshAzureTree(); |
There was a problem hiding this comment.
setSelectedTenantAndSubscriptionIds updates the selectedSubscriptions setting, which (outside Cloud Shell) triggers AzureSubscriptionProvider.onRefreshSuggested and the Azure tree will refresh automatically (see ResourceTreeDataProviderBase.getAzureSubscriptionProvider). Calling ext.actions.refreshAzureTree() here will therefore usually cause an immediate second refresh. Consider instead ensuring the automatic refresh runs with noCache (e.g., set the cache-clear token before/around the setting update) and avoid the explicit refreshAzureTree() to prevent redundant tree reloads.
See below for a potential fix:
// Ensure the automatic Azure tree refresh triggered by the setting update
// runs with a cleared cache so the view reflects the updated selection.
ext.setClearCacheOnNextLoad();
// update the setting (this will trigger the Azure tree to refresh automatically)
await setSelectedTenantAndSubscriptionIds(Array.from(previouslySelectedSubscriptionsSettingValue));
Bug Fixes
#1412 — Re-selecting subscription does not trigger Azure Resources view refresh
PR #1383 removed the explicit
ext.actions.refreshAzureTree()fromselectSubscriptions.ts, assuming the config-change event was sufficient. However, the config-change path doesn't clear the subscription cache, so re-selecting a subscription rendered stale data.Fix: Restore explicit
setClearCacheOnNextLoad()+refreshAzureTree()after updating the subscription selection setting.#1413 — Accounts & Tenants view does not auto-refresh after adding a second account
The cache-clear mechanism was a one-shot boolean — only the first tree to call
consumeClearCacheFlag()receivedtrue. SincelogIn()refreshed Azure Resources before Accounts & Tenants, the Azure tree always consumed the flag, leaving the tenant tree to load from stale cached data (missing the new account).Fix: Convert the one-shot boolean to a counter. Each
setClearCacheOnNextLoad()increments the counter; eachconsumeClearCacheFlag()decrements it and returnstrue. Callers now invokesetClearCacheOnNextLoad()once per tree that needs fresh data.Additional fixes (same pattern)
signInToTenantcommands inregisterCommands.ts(2 places)configureSovereignCloudcommandRoot cause
Both regressions were introduced by PR #1383 ("Performance improvements", commit b76bfd8, Mar 9).
Fixes #1412
Fixes #1413