Skip to content

CmdPal JS/TS Extensions - Phase 4: extension service, wrapper, discovery + host wiring - #49325

Open
michaeljolley wants to merge 17 commits into
dev/mjolley/phase-3-cmdpal-adaptersfrom
dev/mjolley/phase-4-cmdpal-service
Open

CmdPal JS/TS Extensions - Phase 4: extension service, wrapper, discovery + host wiring#49325
michaeljolley wants to merge 17 commits into
dev/mjolley/phase-3-cmdpal-adaptersfrom
dev/mjolley/phase-4-cmdpal-service

Conversation

@michaeljolley

@michaeljolley michaeljolley commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Warning

This PR is one in a series of PRs focused on adding TypeScript/JavaScript extension support to Command Palette.

This PR should not be merged until PR #49326 is merged into it.

Note

To test the final result, run the branch associated with PR #49364.

Summary

Phase 4 of the CmdPal JS/TS extensions effort. This is the integration phase that makes JavaScript/TypeScript Command Palette extensions actually load and run end to end. JS/TS extensions run as isolated Node.js processes that talk to the C# host over JSON-RPC 2.0 via stdio (LSP Content-Length framing).

Tracking issue: #48707

Stacking

This PR stacks on Phase 3 (#49324) and targets dev/mjolley/phase-3-cmdpal-adapters, not main. It stays a draft until the earlier phases are reviewed and merged. Phases 1-3 (TypeScript SDK, JSON-RPC transport + manifest, and the adapter/proxy translation layer) are already present in the base branch.

What this adds

  • Models/JSExtensionWrapper.cs (IExtensionWrapper): spawns node <entrypoint> with stdio redirection, honoring cmdpal.debug to pass --inspect and debugPort (auto-assigned from 9229). Owns a JsonRpcConnection (Phase 2), sends the initialize handshake, and exposes the JSCommandProviderProxy (Phase 3). Lifecycle start/stop/dispose sends the dispose notification with a 2 second grace period before killing the process tree. Teardown is deadlock-safe (handlers unsubscribed and fields captured under lock, blocking work done outside the lock). A consecutive-crash counter treats 1-3 crashes as restartable and marks the extension unhealthy after more than 3.
  • Services/JsonRpcExtensionService.cs (IExtensionService): discovers extensions under %LOCALAPPDATA%\Microsoft\PowerToys\CommandPalette\JSExtensions, validates each package.json against the Phase 2 manifest, and builds a CommandProviderWrapper per extension. A FileSystemWatcher handles install (load), uninstall (unload + kill), and *.js changes for hot-reload (debounced 500 ms, ignoring node_modules). Raises OnProviderAdded/OnProviderRemoved.
  • Services/HotReloadDebouncer.cs: testable 500 ms per-directory debounce that ignores node_modules changes.
  • CommandProviderWrapper.cs: new constructor for the JS path, where the ICommandProvider is obtained directly over JSON-RPC instead of through GetExtensionObject.
  • Wiring (App.xaml.cs): registers JsonRpcExtensionService as an IExtensionService singleton alongside the built-in and WinRT services. TopLevelCommandManager consumes it through the existing IEnumerable<IExtensionService> path, and MainWindow disposes it through the existing SignalStopAsync loop, so no new signatures were needed.

Host-side host/logMessage, host/showStatus, host/hideStatus, and host/copyText notifications are already handled by the JSCommandProviderProxy from Phase 3, so no ShellViewModel/MainWindow host-notification changes were required.

How JS extensions now load end to end

  1. On startup the service scans the JSExtensions directory and validates each manifest.
  2. For each valid extension it constructs a JSExtensionWrapper, spawns the Node.js process, performs the initialize handshake, and wraps the resulting provider in a CommandProviderWrapper.
  3. TopLevelCommandManager picks the providers up through the shared IExtensionService enumeration and surfaces their commands.
  4. Directory and source-file watchers keep the set live: new directories load, deleted directories unload and kill the process, and *.js edits hot-reload (recreating the wrapper so the proxy rebinds to a fresh connection).
  5. On shutdown each extension receives a dispose notification followed by process-tree termination.

Validation

  • Build: tools\build\build.ps1 -Path src\modules\cmdpal -Platform x64 -Configuration Debug exits 0.
  • Unit tests: all CmdPal *.UnitTests projects pass. New tests cover discovery/manifest filtering, hot-reload debounce (including node_modules exclusion and cancellation), and crash-counter state transitions, all factored to run without spawning a Node.js process.

…ery + host wiring

Makes JavaScript/TypeScript CmdPal extensions load and run end to end.

- JSExtensionWrapper: spawns `node <entrypoint>` (honoring cmdpal.debug ->
  --inspect and debugPort, auto-assigning from 9229), owns a JsonRpcConnection,
  sends the initialize handshake, and exposes the JSCommandProviderProxy.
  Lifecycle start/stop/dispose sends the dispose notification with a 2s grace
  period before killing the process tree. Deadlock-safe teardown and a
  consecutive-crash counter (1-3 restartable, >3 marks the extension unhealthy).
- JsonRpcExtensionService (IExtensionService): discovers extensions under
  %LOCALAPPDATA%\Microsoft\PowerToys\CommandPalette\JSExtensions, builds a
  CommandProviderWrapper per extension, and watches the directory for install,
  uninstall, and *.js changes for hot-reload (debounced 500ms, ignoring
  node_modules). Raises OnProviderAdded/OnProviderRemoved.
- HotReloadDebouncer: testable 500ms per-directory debounce that ignores
  node_modules changes.
- CommandProviderWrapper: new constructor for the JS path where the provider is
  obtained directly over JSON-RPC rather than through GetExtensionObject.
- App.xaml.cs: registers JsonRpcExtensionService as an IExtensionService
  singleton alongside the built-in and WinRT services; TopLevelCommandManager
  consumes it through the existing IEnumerable<IExtensionService> path.

Host-side host/logMessage, host/showStatus, host/hideStatus, and host/copyText
notifications are handled by the existing JSCommandProviderProxy (Phase 3), so no
ShellViewModel/MainWindow changes were required.

Tests: discovery/manifest filtering, hot-reload debounce, and crash-counter
transitions, all factored to run without spawning a Node.js process.

Co-authored-by: Copilot App <[email protected]>
Copilot-Session: a6ddb4d4-165a-40a5-b430-7b66739de49e
michaeljolley and others added 6 commits July 15, 2026 11:16
…ers' into dev/mjolley/phase-4-cmdpal-service
Fix crash handling, shutdown race, and hot-reload directory matching.

High: An unexpected Node.js exit now tears down the process and connection
handles and notifies the service via a new JSExtensionWrapper.ProcessExited
event, raised off the read-loop thread to avoid self-joining the connection
dispose. The service removes the dead provider, tracks per-directory
consecutive crash counts, and restarts up to three times before disabling the
extension (leaving the directory watcher active so a reinstall gets a fresh
chance).

Medium: A shutdown CancellationTokenSource is cancelled at the start of
SignalStopAsync and in Dispose. The delayed directory-created handler and
LoadExtensionAsync bail out when cancellation is requested, so a queued handler
can no longer start an extension after shutdown has completed.

Low: Hot-reload directory matching now uses a directory-boundary containment
check (IsUnderDirectory) instead of an unbounded string prefix, so changes
under foo-bar no longer restart extension foo.

Adds unit tests for DecideCrashAction and IsUnderDirectory.

Co-authored-by: Copilot App <[email protected]>
Copilot-Session: a6ddb4d4-165a-40a5-b430-7b66739de49e
…selection fixes

Fix four host defects surfaced while dogfooding JS/TS extensions:

- showStatus indeterminate progress was dropped. HandleShowStatusNotification
  now reads the SDK progress payload and sets StatusMessage.Progress via the
  toolkit ProgressState, so an indeterminate spinner renders.
- Status severity always rendered as Info. The state reader now honors the
  Pascal-case State the SDK nests in the message object (as well as the
  camelCase state used by logMessage), mapping success/warning/error correctly.
- Opening a JS image page crashed with ArgumentOutOfRangeException. The Unbound
  icon size maps to Size.Empty (Width/Height NegativeInfinity); scaling now
  short-circuits when the size is empty so no negative Size is constructed,
  preserving the existing no-resize behavior.
- Selecting a non-first item on a live-updating details page snapped back to the
  first item. The JS list proxy now reuses list-item adapters by stable identity
  across refreshes and updates their payload in place, so the host keeps the same
  view model and preserves the current selection.

Add unit tests for the status wire-shape parsing (indeterminate progress,
determinate percent, and severity mapping).

Co-authored-by: Copilot App <[email protected]>
Copilot-Session: 419a1624-b0e2-439e-bf78-dacf6e126c06
Align the JS extensions install directory with the CmdPal data and log
root by changing the LocalAppData path segment from CommandPalette to
CmdPal, matching where CmdPal writes its logs.

Co-authored-by: Copilot App <[email protected]>
Copilot-Session: f69b1dcf-ed86-4e74-8373-b8414e1ac961
@michaeljolley
michaeljolley requested review from jiripolasek and zadjii-msft and removed request for jiripolasek July 21, 2026 17:05
@michaeljolley michaeljolley self-assigned this Jul 21, 2026
@michaeljolley
michaeljolley marked this pull request as ready for review July 21, 2026 17:06
michaeljolley and others added 10 commits July 21, 2026 13:26
…capabilities

Change A: JSExtensionManifest.Publisher keeps cmdpal.publisher as the primary
source and now falls back to the name portion of the top-level npm "author"
field when cmdpal.publisher is absent or whitespace. The npm author field is
modeled as a JsonElement so both the string form ("Name email url") and the
object form with name, email, and url properties are supported; only the name
is extracted. JSExtensionWrapper still coalesces a null publisher to "Unknown".

Change B: Remove the manifest-level cmdpal.capabilities field. It mapped only
"commands" to ProviderType.Commands, which is also the default for the
empty/absent case, so the field had no observable effect. The wrapper now
simply advertises ProviderType.Commands. The unrelated runtime initialize
handshake capabilities (RecordAdvertisedCapabilities and the internal
Capabilities property) are left intact.

Co-authored-by: Copilot App <[email protected]>
Copilot-Session: de2fe464-6835-4cd9-9c6b-0ac9351cdb99
Co-authored-by: Copilot App <[email protected]>
Copilot-Session: 762976e0-2453-40ca-8034-978a04a19d1e
Co-authored-by: Copilot App <[email protected]>
Copilot-Session: 3f0875f9-177f-40db-888c-248a044d2bb9
Co-authored-by: Copilot App <[email protected]>

Copilot-Session: 9470f549-1997-44de-b0a6-1ce4562a51f0
Implements the phase-4 merge blockers p4-01 through p4-08 and wires the three
phase-3 checkpoints cp-A, cp-B, and cp-C.

- p4-01 replaceable reload cancellation so a stop then a later load reloads
  providers instead of reusing a permanently canceled token.
- p4-02 per canonical directory lifecycle gate that serializes refresh, crash
  restart, hot reload, and initial load; a duplicate loser is disposed.
- p4-03 stable list item identity keyed by stable id or command id across a
  refresh so a duplicate title reorder keeps each row bound to its command.
- p4-04 robust discovery: manifest create, replace, and rename watching, a
  manifest stability retry, and a reconciliation scan for installs and removals.
- p4-05 host notification handlers registered and buffered before activation so
  startup logs, statuses, and clipboard requests are delivered.
- p4-06 status updates applied in place, removal by id independent of severity,
  and cleanup of active statuses when the extension disconnects.
- p4-07 deterministic cross extension duplicate id policy that picks the
  canonical path sorted winner and rejects the rest.
- p4-08 unit tests covering the above.
- cp-A the service disposes the provider proxy during teardown so its handlers
  are released while the wrapper still owns process teardown.
- cp-B the initialize handshake provider metadata is threaded into the proxy so
  the real author frozen value flows through.
- cp-C page state is left on the item response piggyback, noted rather than
  gold plated.

Co-authored-by: Copilot App <[email protected]>
Copilot-Session: e829f345-adca-4d06-a658-6cf416c3722e
Propagates the stale-test and golden-fixture corrections so the SDK
typecheck stays green at this layer.

Co-authored-by: Copilot App <[email protected]>
Copilot-Session: 311db8aa-f8db-47bb-943f-5447037946c7
Round 2 fixes for the Command Palette JS/TS extension host lifecycle,
scoped to src/modules/cmdpal.

- r2-p4-01: make provider-id reservation atomic and shared across every
  registration path. A single synchronized ProviderIdReservations map is
  consulted-and-claimed under _extensionsLock before any provider registers,
  so a full scan, hot reload, or concurrent install can never register two
  providers with the same id. The id is released on every removal path
  (uninstall, hot-reload, crash) and re-reserved on add.
- r2-p4-02: keep a directory's lifecycle gate entry alive while any caller
  holds or awaits it. Remove now marks the entry removed and evicts it only
  once its last reference drains, so a new generation strictly supersedes the
  prior one and cannot overlap it.
- r2-p4-03: after init completes, probe the child process and drive the crash
  path if it already exited, so an immediate post-init crash is handled rather
  than registered as healthy.
- r2-p4-04: make the root directory watcher segment-aware so churn under
  node_modules and .git subtrees no longer triggers reload storms.
- r2-p4-05: subscribe the source watcher to Renamed and Deleted (not just
  in-place writes) and route both the old and new paths, so atomic
  temp-file-plus-rename saves and deletions reload.
- r2-p4-06: an explicit refresh now re-reads manifests and hot-reloads any
  loaded extension whose manifest changed, instead of only re-enumerating
  directories.
- r2-p4-07: a crash-disabled extension keeps its source watcher alive and a
  source edit hot-reloads it, resetting the crash count so it recovers instead
  of being stranded until reinstall.
- r2-p4-08: order status hide against disposal under the host lock so a show
  can never resurrect or strand host UI after teardown.

Adds deterministic, node-free unit tests for each fix.

Co-authored-by: Copilot App <[email protected]>
Copilot-Session: 311db8aa-f8db-47bb-943f-5447037946c7
Bring the phase-3 adapter round-2 fixes down onto phase-4. Resolved JSCommandProviderProxy.cs status-disposal overlap by keeping phase-4 _hostLock/RunWithHost, which subsumes the phase-3 _statusLock fix.

Co-authored-by: Copilot App <[email protected]>
Copilot-Session: 311db8aa-f8db-47bb-943f-5447037946c7
@michaeljolley
michaeljolley force-pushed the dev/mjolley/phase-4-cmdpal-service branch from ca60ab1 to ca9c628 Compare July 27, 2026 21:14
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