Skip to content

CmdPal: Refactor TopLevelCommandManager to use IExtensionService implementations#48417

Open
michaeljolley wants to merge 9 commits into
mainfrom
michaeljolley/refactor-tlcm-extension-services
Open

CmdPal: Refactor TopLevelCommandManager to use IExtensionService implementations#48417
michaeljolley wants to merge 9 commits into
mainfrom
michaeljolley/refactor-tlcm-extension-services

Conversation

@michaeljolley

@michaeljolley michaeljolley commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Summary

The TopLevelCommandManager handles alot. And by "alot" I mean too much. In preparation for future types of extensions, this PR attempts to pull out the methods for loading/managing extensions to individual IExtensionServices.

These ExtensionServices will be injected the TopLevelCommandManager via dependency injection. It will iterate through them asking them to load their extensions. During that process, they will surface ICommandProviders back to the TopLevelCommandManager for display in Command Palette.

Currently, there are two types of IExtensionServices: BuiltInExtensionService and WinRTExtensionService.

  • The BuiltInExtensionService receives all ICommandProviders registered with DI.
  • The WinRTExtensionService manages out-of-process WinRT AppExtension providers.

Additional Changes

  • Circular DI dependency resolved: BuiltInsCommandProvider no longer depends on IRootPageService.
    Previously, it required the IRootPageService solely for the dock command to open Command Palette (using the RootPage as its command.) Now, it uses a new GoHomeDockCommand which returns CommandResult.GoHome() with the existing onBeforeShowConfirmation callback to show the palette at the dock button position.

Extract extension loading from TopLevelCommandManager into separate
IExtensionService implementations:

- BuiltInExtensionService: wraps in-proc ICommandProvider instances from DI
- WinRTExtensionService: manages out-of-process WinRT AppExtension providers

TLCM now receives IEnumerable<IExtensionService> and orchestrates them
uniformly. This reduces TLCM from ~900 to ~700 lines and separates
provider-specific concerns (timeout, retry, catalog events) from
command orchestration.

Additional fixes:
- Break circular DI dependency: BuiltInsCommandProvider no longer needs
  IRootPageService. Uses GoHomeDockCommand (returns CommandResult.GoHome())
  with onBeforeShowConfirmation callback to show palette at dock position.
- Restore two-phase loading: PreLoadAsync loads only built-ins (instant),
  PostLoadRootPageAsync loads WinRT extensions on background thread.
- Add try/finally around IsLoading to prevent stuck loading indicator.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

michaeljolley and others added 2 commits June 9, 2026 21:45
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…lass

- Use IExtensionWrapper directly (with existing using) in
  ExtensionGalleryViewModel instead of fully-qualified name
- Replace ct.ThrowIfCancellationRequested() with graceful early return
  in BuiltInExtensionService.LoadProvidersAsync
- Extract ExtensionStartResult into its own file
- Restore 'Go back to the main page, but keep it open' comment

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@michaeljolley michaeljolley added the Product-Command Palette Refers to the Command Palette utility label Jun 10, 2026
@michaeljolley michaeljolley marked this pull request as ready for review June 12, 2026 23:23
@michaeljolley michaeljolley enabled auto-merge (squash) June 12, 2026 23:24
@microsoft microsoft deleted a comment from github-actions Bot Jun 13, 2026
@github-actions

This comment has been minimized.

@michaeljolley michaeljolley marked this pull request as draft June 13, 2026 02:50
auto-merge was automatically disabled June 13, 2026 02:50

Pull request was converted to draft

@michaeljolley michaeljolley marked this pull request as ready for review June 13, 2026 03:08
@michaeljolley michaeljolley enabled auto-merge (squash) June 13, 2026 03:08
@github-actions

Copy link
Copy Markdown

@check-spelling-bot Report

🔴 Please review

See the 📂 files view, the 📜action log, 👼 SARIF report, or 📝 job summary for details.

❌ Errors and Warnings Count
⚠️ binary-file 1
⚠️ duplicate-pattern 2
❌ forbidden-pattern 1
⚠️ large-file 1

See ❌ Event descriptions for more information.

These words are not needed and should be removed nullability

Some files were automatically ignored 🙈

These sample patterns would exclude them:

^src/modules/ZoomIt/ZoomIt/rnnoise/rnnoise_data_little\.c$
^src/modules/ZoomIt/ZoomIt/selfie_segmentation\.onnx$

You should consider adding them to:

.github/actions/spell-check/excludes.txt

File matching is via Perl regular expressions.

To check these files, more of their words need to be in the dictionary than not. You can use patterns.txt to exclude portions, add items to the dictionary (e.g. by adding them to allow.txt), or fix typos.

To update file exclusions and remove the previously acknowledged and now absent words, you could run the following commands

... in a clone of the git@github.com:microsoft/PowerToys.git repository
on the michaeljolley/refactor-tlcm-extension-services branch (ℹ️ how do I use this?):

curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/cfb6f7e75bbfc89c71eaa30366d0c166f1bd9c8c/apply.pl' |
perl - 'https://github.com/microsoft/PowerToys/actions/runs/27454773407/attempts/1' &&
git commit -m 'Update check-spelling metadata'

OR

To have the bot accept them for you, comment in the PR quoting the following line:
@check-spelling-bot apply updates.

Forbidden patterns 🙅 (1)

In order to address this, you could change the content to not match the forbidden patterns (comments before forbidden patterns may help explain why they're forbidden), add patterns for acceptable instances, or adjust the forbidden patterns themselves.

These forbidden patterns matched content:

Should be a
\san (?=(?:[b-dfgjklpqtvwz]|h(?!onou?r|our|s[lv]|tml|ttp|ref)|n(?!ginx|grok|pm)|r(?!c)|s(?!s[ho]|vg))[a-z]|x(?!\b|[-\d]|ml))
If the flagged items are 🤯 false positives

If items relate to a ...

  • binary file (or some other file you wouldn't want to check at all).

    Please add a file path to the excludes.txt file matching the containing file.

    File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

    ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

  • well-formed pattern.

    If you can write a pattern that would match it,
    try adding it to the patterns.txt file.

    Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

    Note that patterns can't match multiline strings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Product-Command Palette Refers to the Command Palette utility

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants