Fix CmdPal extension installed state detection for Store-only package…#48746
Open
namdpran8 wants to merge 6 commits into
Open
Fix CmdPal extension installed state detection for Store-only package…#48746namdpran8 wants to merge 6 commits into
namdpran8 wants to merge 6 commits into
Conversation
github-actions
Bot
added
the
Product-Command Palette
Contributor
Author
|
@microsoft-github-policy-service agree |
… view model logic
github-actions
Bot
added
the
Area-Localization
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves Command Palette (CmdPal) extension “installed” state detection in the Extension Gallery by adding a third fallback that can resolve Store-only extensions (those without PFN and WinGet package ID) via WinGet’s Microsoft Store catalog.
Changes:
- Refactors gallery install-state checking to avoid early returns and adds a StoreId-based “Pass 3” detection path.
- Extends the WinGet package manager service with a Store-ID lookup API and updates composite catalog search behavior to enable correlating remote packages to local installed state.
- Updates the WinGet service interface to expose the new Store-ID lookup method.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Gallery/ExtensionGalleryViewModel.cs | Adds Pass 3 StoreId detection and adjusts control flow so it can run even when WinGet IDs are absent. |
| src/modules/cmdpal/Microsoft.CmdPal.Common/WinGet/Services/WinGetPackageManagerService.cs | Implements StoreId lookups via the WinGet composite catalog and changes composite search behavior to include local correlation. |
| src/modules/cmdpal/Microsoft.CmdPal.Common/WinGet/Services/IWinGetPackageManagerService.cs | Adds the new GetStorePackagesByIdAsync API contract. |
Comment on lines
+472
to
+525
| // Pass 3 (WinGet Store Catalog Search) | ||
| if (_winGetPackageManagerService is not null && _winGetPackageManagerService.State.IsAvailable) | ||
| { | ||
| lock (_entriesLock) | ||
| { | ||
| snapshot = [.. _allEntries]; | ||
| } | ||
|
|
||
| var wingetIds = snapshot | ||
| .Select(entry => entry.WinGetId) | ||
| .Where(static id => !string.IsNullOrWhiteSpace(id)) | ||
| .Distinct(StringComparer.OrdinalIgnoreCase) | ||
| .Cast<string>() | ||
| .ToArray(); | ||
| if (wingetIds.Length == 0) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| using var wingetCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); | ||
| wingetCts.CancelAfter(WinGetRefreshTimeout); | ||
| var wingetInfos = await RunInBackgroundAsync( | ||
| () => _winGetPackageStatusService.TryGetPackageInfosAsync(wingetIds, wingetCts.Token), | ||
| wingetCts.Token); | ||
| wingetCts.Token.ThrowIfCancellationRequested(); | ||
| if (wingetInfos is null) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| foreach (var entry in snapshot) | ||
| try | ||
| { | ||
| if (string.IsNullOrWhiteSpace(entry.WinGetId)) | ||
| lock (_entriesLock) | ||
| { | ||
| continue; | ||
| snapshot = [.. _allEntries]; | ||
| } | ||
|
|
||
| if (!wingetInfos.TryGetValue(entry.WinGetId, out var packageInfo)) | ||
| var storeIdsToLookup = snapshot | ||
| .Where(e => !e.IsInstalledStateKnown && !string.IsNullOrWhiteSpace(e.StoreId)) | ||
| .Select(e => e.StoreId!) | ||
| .Distinct(StringComparer.OrdinalIgnoreCase) | ||
| .ToList(); | ||
|
|
||
| if (storeIdsToLookup.Count > 0) | ||
| { | ||
| continue; | ||
| using var storeCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); | ||
| storeCts.CancelAfter(WinGetRefreshTimeout); | ||
|
|
||
| var results = await RunInBackgroundAsync( | ||
| () => _winGetPackageManagerService.GetStorePackagesByIdAsync(storeIdsToLookup, storeCts.Token), | ||
| storeCts.Token); | ||
| storeCts.Token.ThrowIfCancellationRequested(); | ||
|
|
||
| if (results?.Value != null) | ||
| { | ||
| foreach (var entry in snapshot) | ||
| { | ||
| if (entry.IsInstalledStateKnown || string.IsNullOrWhiteSpace(entry.StoreId)) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| if (results.Value.TryGetValue(entry.StoreId, out var catalogPackage)) | ||
| { | ||
| entry.IsInstalled = catalogPackage.InstalledVersion != null; | ||
| entry.IsInstalledStateKnown = true; | ||
| } | ||
| } | ||
|
|
||
| // Mark any Store-ID entries not found in the catalog as known-not-installed. | ||
| foreach (var entry in snapshot) | ||
| { | ||
| if (!entry.IsInstalledStateKnown && !string.IsNullOrWhiteSpace(entry.StoreId)) | ||
| { | ||
| entry.IsInstalled = false; | ||
| entry.IsInstalledStateKnown = true; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| QueueApplyFilter(); |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
jiripolasek
reviewed
Jul 8, 2026
jiripolasek
left a comment
jiripolasek
left a comment
Collaborator
There was a problem hiding this comment.
I've some nits and questions about this:
|
|
||
| var options = initialization.Factory.CreateCreateCompositePackageCatalogOptions(); | ||
| options.CompositeSearchBehavior = CompositeSearchBehavior.RemotePackagesFromAllCatalogs; | ||
| options.CompositeSearchBehavior = CompositeSearchBehavior.AllCatalogs; |
Collaborator
There was a problem hiding this comment.
This worries me as this is a shared service. What's the impact on other consumers?
Contributor
Author
There was a problem hiding this comment.
Thanks for pointing that out. I've reverted the default behavior back to RemotePackagesFromAllCatalogs to avoid impacting existing consumers. Instead, I introduced an overload that accepts a CompositeSearchBehavior parameter so callers can explicitly opt into AllCatalogs when needed. I also updated the caching logic to maintain separate cached composite catalogs for the different search behaviors.
|
|
||
| return new WinGetQueryResult<IReadOnlyDictionary<string, CatalogPackage>>(results, false, null); | ||
| } | ||
| catch (Exception ex) when (ex is COMException or InvalidOperationException or TaskCanceledException) |
Collaborator
There was a problem hiding this comment.
OperationCanceledException should be handled before this catch to make it consistent with the rest of the class.
There's also pre-existing pattern and code smell with this catch cascade:
catch (OperationCanceledException)
catch (Exception ex) when (ex is TaskCancelledException)
Since the TaskCanceledException is subclass of OperationCanceledException, that's unreachable.
Contributor
Author
There was a problem hiding this comment.
Fixed — I added catch (OperationCanceledException) and removed the unreachable TaskCanceledException from the when clause in GetStorePackagesByIdAsync.
I also noticed the same exception handling pattern exists in a few other places in this class:
InstallPackageAsyncandUninstallPackageAsyncalready have anOperationCanceledExceptioncatch, but still includeTaskCanceledExceptionin the subsequentwhenfilter.SearchPackagesAsync,GetPackagesByIdAsync,RefreshCatalogsAsync, andCreateCompositeCatalogAsyncdon't have a dedicatedOperationCanceledExceptioncatch.
Would you prefer that I clean up those occurrences in this PR as well for consistency, or keep this PR scoped to the current change?
- Reuse a single snapshot across all installation detection passes. - Add an explicit OperationCanceledException catch. - Remove the redundant TaskCanceledException check from the exception filter.
Comment on lines
529
to
533
| var result = await task.ConfigureAwait(false); | ||
| if (!result.IsSuccess || result.Value is null) | ||
| { | ||
| ClearCachedCompositeCatalogTask(includeStoreCatalog, task); | ||
| ClearCachedCompositeCatalogTask(includeStoreCatalog, searchBehavior, task); | ||
| } |
Comment on lines
+239
to
+243
| var selector = initialization.Factory.CreatePackageMatchFilter(); | ||
| selector.Field = PackageMatchField.Id; | ||
| selector.Option = PackageFieldMatchOption.Equals; | ||
| selector.Value = id.ToUpperInvariant(); | ||
| options.Selectors.Add(selector); |
Comment on lines
+246
to
+249
| if (findResult.Status != FindPackagesResultStatus.Ok) | ||
| { | ||
| throw new InvalidOperationException($"Microsoft Store package lookup failed for '{id}': {findResult.Status}"); | ||
| } |
Comment on lines
+499
to
+507
| // Mark any Store-ID entries not found in the catalog as known-not-installed. | ||
| foreach (var entry in snapshot) | ||
| { | ||
| if (!entry.IsInstalledStateKnown && !string.IsNullOrWhiteSpace(entry.StoreId)) | ||
| { | ||
| entry.IsInstalled = false; | ||
| entry.IsInstalledStateKnown = true; | ||
| } | ||
| } |
Comment on lines
+51
to
+53
| Task<WinGetQueryResult<IReadOnlyDictionary<string, CatalogPackage>>> GetStorePackagesByIdAsync( | ||
| IEnumerable<string> storeIds, | ||
| CancellationToken cancellationToken = default); |
Comment on lines
551
to
553
| cancellationToken.ThrowIfCancellationRequested(); | ||
|
|
||
| var options = initialization.Factory.CreateCreateCompositePackageCatalogOptions(); |
…cancellation requests
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of the Pull Request
Store-only Command Palette extensions (e.g., Process Killer, ADB Extension, Browser Tabs) lacked both a WinGet package ID and a Package Family Name (PFN), meaning they were never detected as installed in the Extension Gallery—even when actively running on the system. This PR adds a third fallback detection pass that queries the WinGet composite catalog using Store IDs, successfully marking these extensions as installed when a local installation is found
galleryviewbug_1.mp4
Recording 1: Reproducing the original bug (Store-only extensions showing as not installed).
PR Checklist
Detailed Description of the Pull Request / Additional comments
fix2.mp4
Recording 2: Running cmdpal with the bug fixed (Extensions correctly marked as installed).
1. Store Package Lookup Service
IWinGetPackageManagerService.cs&WinGetPackageManagerService.cs:GetStorePackagesByIdAsync(IEnumerable<string> storeIds, ...)to resolve Microsoft Store packages using the WinGet composite catalog.SemaphoreSlim(4)) to avoid overloading the WinGet engine while keeping lookup latency low.GetCompositeCatalogResultAsync/CreateCompositeCatalogAsyncthat accepts aCompositeSearchBehavior.GetStorePackagesByIdAsyncusesCompositeSearchBehavior.AllCatalogs, allowing WinGet to populateInstalledVersionand reliably determine whether Store-only extensions are installed. Existing callers (SearchPackagesAsync,GetPackagesByIdAsync) continue usingRemotePackagesFromAllCatalogs. Composite catalogs are cached separately for each search behavior to avoid incorrect cache reuse.2. Gallery ViewModel Integration
ExtensionGalleryViewModel.cs:CheckInstalledAsync()to avoid early returns if there are no WinGet package IDs to check, ensuring the new Pass 3 for Store-only packages is allowed to run.StoreIdbut whose installation state is not yet verified.GetStorePackagesByIdAsynchelper and marks the items as installed or not based on whether the returnedCatalogPackage.InstalledVersionis non-null.Detection order now becomes:
Note on caching lag:
Due to existing
PackageManagercaching, extensions installed while CmdPal is running may not be detected until CmdPal is restarted or WinGet refreshes its index.Long-term data-side fix
The proper permanent fix for Store-only packages is to add
"detection": { "packageFamilyName": "<PFN>" }entries to the remoteextensions.jsonin the microsoft/CmdPal-Extensions repository. When that field is present, Pass 1 resolves the installed state instantly without any WinGet involvement. Pass 3 acts as a fallback for extensions that do not yet have a PFN defined in the feedFix1.mp4
Recording 3: Demonstration that adding a PFN to the extension feed allows Pass 1 detection without the fallback path.
Validation Steps Performed
9NZ06M9CNV77) now correctly show as Installed when the package is present on the system.