CmdPal JS/TS Extensions - Phase 5: gallery npm install/uninstall - #49326
Open
michaeljolley wants to merge 17 commits into
Open
CmdPal JS/TS Extensions - Phase 5: gallery npm install/uninstall#49326michaeljolley wants to merge 17 commits into
michaeljolley wants to merge 17 commits into
Conversation
Recognize gallery entries of type jsonrpc (npm package + optional registry) and install/uninstall them into the Phase 4 JSExtensions directory so the FileSystemWatcher loads/unloads them. - Add GalleryNpmPackage model and Npm block on GalleryInstallSource. - Expose the shared JSExtensions root and process termination from JsonRpcExtensionService via IJsExtensionHost. - Add IJsExtensionInstaller/NpmJsExtensionInstaller and INpmCommandRunner/NpmCommandRunner as mockable seams; guard against npm not being installed with a user-visible error. - Wire jsonrpc install/uninstall commands, state, and UI into the gallery item view model and page. - Add unit tests for jsonrpc entry classification, install/uninstall command behavior, and installer path resolution plus stop-before-delete sequencing. Co-authored-by: Copilot App <[email protected]> Copilot-Session: 1fac744e-2d8a-4f58-9044-6a1565b19b37
github-actions
Bot
added
the
Product-Command Palette
…ce' into dev/mjolley/phase-5-cmdpal-gallery # Conflicts: # src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/JsonRpcExtensionService.cs
Fixes three gallery install/uninstall review notes: - Uninstall now surfaces directory-deletion failure. RemoveDirectory returns a bool and UninstallAsync reports failure (instead of Ok) when the extension files could not be removed, so the UI no longer marks an extension uninstalled while it remains on disk. - Install now verifies the extension is discoverable before reporting success. After npm exits, IsExtensionDiscoverable checks for a loadable manifest at the directory root; if absent, the directory is cleaned up and a clear error is returned. This keeps success honest and composes with the Phase 6 layout materialization. - npm install now has a five minute timeout. A linked cancellation source bounds WaitForExitAsync, kills the process tree on timeout, and returns a clear error, so the gallery cannot stay on Installing indefinitely. External cancellation is still distinguished and rethrown. Co-authored-by: Copilot App <[email protected]> Copilot-Session: 1fac744e-2d8a-4f58-9044-6a1565b19b37
Align the hardcoded expected install root in the npm installer tests with the CmdPal data folder path segment change from CommandPalette to CmdPal. Co-authored-by: Copilot App <[email protected]> Copilot-Session: f69b1dcf-ed86-4e74-8373-b8414e1ac961
This was referenced Jul 21, 2026
michaeljolley
marked this pull request as ready for review
July 21, 2026 17:06
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
…-gallery # Conflicts: # src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/JsonRpcExtensionService.cs
…hase 5) Rework the Command Palette JS extension gallery install/uninstall flow into a transaction over an immutable, approved artifact per the C5 contract. Artifact identity and validation (NpmArtifact): - Require package (npm name grammar), exact version (no ranges or dist-tags), sha512 Subresource-Integrity, and an optional absolute HTTPS registry on an approved allowlist. Incomplete catalog entries are not installable. - Install spec is always name@exactversion, passed via ArgumentList, and can never be read as a flag, URL, path, git, or tarball source. Runner (NpmCommandRunner): - npm always runs with --ignore-scripts --save-exact --no-audit --no-fund --loglevel=error so package lifecycle scripts never run. - Read the resolved integrity from the lockfile for post-install verification. - RemoveDirectory refuses reparse points and retries with a bounded backoff. Installer (NpmJsExtensionInstaller): - Stage into a unique dir outside the watched root, verify integrity, identity, and version, assemble the discovery layout, then atomically promote with Directory.Move and await host provider registration before reporting success. - Block reinstalling into an existing or active directory (upgrade policy). - Serialize install and uninstall of the same canonical directory; allow different directories to run in parallel. - Uninstall stops the extension, then deletes with reparse and canonical containment checks. Staging is always cleaned up in a finally block. Host (IJsExtensionHost, JsonRpcExtensionService): - Add IsExtensionInstalled and RefreshAndAwaitProviderAsync so installed state is derived from a loadable manifest in the host, not the catalog. View model and UI: - Seed IsInstalled from the host, pass version, integrity, and a cancellation token, and expose a cancel affordance mirroring the WinGet flow. - All user-facing install and uninstall messages come from Resources.resx. Add tests for the artifact validation matrix, argument construction, reparse refusal, scoped and unscoped layouts, delayed discovery, integrity and version verification, registry rejection, lifecycle-script disabling, rollback on timeout and cancel, existing-install preservation, and concurrency. Co-authored-by: Copilot App <[email protected]> Copilot-Session: 234c59f1-a32b-4c98-97bb-ab50a4d07fd9
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
Address the blocking command-injection defect and related install/uninstall robustness issues in the Command Palette JS extension gallery installer: r2-p5-01 (BLOCK): canonicalize the npm registry to a strict https origin (approved host, no userinfo/port/query/fragment/path) and invoke npm through node.exe with npm-cli.js instead of the npm.cmd batch shim, so untrusted arguments never reach cmd.exe. r2-p5-02: gate installs on a lockfile-integrity check that requires every resolved dependency to be registry-sourced https with an SRI hash, failing closed otherwise. r2-p5-03: on canceled or failed installs, stop the host provider before removing the promoted directory on every rollback path. r2-p5-04: refuse uninstall when the extensions root resolves through a reparse point outside its expected location. r2-p5-05: report a package as installed when present on disk even if the provider failed to load, so corrupt installs can be uninstalled. r2-p5-06: thread the uninstall cancellation token through the stop and delete steps. Co-authored-by: Copilot App <[email protected]> Copilot-Session: 311db8aa-f8db-47bb-943f-5447037946c7
Bring the phase-4 service round-2 fixes down onto phase-5. Resolved JsonRpcExtensionService.cs as a union: kept the phase-5 installer members and the phase-4 RegistrationOutcome enum plus its register-block usage. Co-authored-by: Copilot App <[email protected]> Copilot-Session: 311db8aa-f8db-47bb-943f-5447037946c7
michaeljolley
force-pushed
the
dev/mjolley/phase-4-cmdpal-service
branch
from
July 27, 2026 21:14
ca60ab1 to
ca9c628
Compare
michaeljolley
force-pushed
the
dev/mjolley/phase-5-cmdpal-gallery
branch
from
July 27, 2026 21:14
8d8c090 to
a20c022
Compare
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.
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 #49329 is merged into it.
Note
To test the final result, run the branch associated with PR #49364.
Summary
Phase 5 of the CmdPal JS/TS extensions effort (tracking issue #48707). This adds gallery install/uninstall for JavaScript/TypeScript (jsonrpc) extensions.
Stacks on Phase 4 (PR #49325). Base branch is
dev/mjolley/phase-4-cmdpal-service, notmain. Stays a draft until reviewed.What this does
type: jsonrpcwith a nestednpmblock ({ package, registry }), distinct from the existing COM/WinGet/Store/url sources.npm install <package>intoJSExtensions/<name>/(the shared Phase 4 path, exposed viaIJsExtensionHost.ExtensionsRootPath). Optionalregistryis honored. Progress/errors surface in the gallery item UI, consistent with the existing install UX.IJsExtensionHost.StopExtension, backed by the Phase 4 service) then deletesJSExtensions/<name>/. The Phase 4FileSystemWatcherhandles the actual load/unload; this layer does not mutate provider lists directly.Design
INpmCommandRunner/NpmCommandRunnerisolate the real npm process and directory side effects.IJsExtensionInstaller/NpmJsExtensionInstallerorchestrate install/uninstall, with a path-traversal guard that keeps the target directory inside the JSExtensions root.Tests
ExtensionGalleryItemViewModelTests: jsonrpc entry classification (HasJsonRpcSource,JsonRpcPackageId,JsonRpcRegistry, source appears inSources) and install/uninstall command behavior via a mockedIJsExtensionInstaller.NpmJsExtensionInstallerTests: target-directory resolution (root + name), npm-not-found guard, cleanup on failure, path-traversal rejection, and stop-before-delete sequencing on uninstall.*.UnitTestspass.Known caveat
npm install <package>places the package underJSExtensions/<name>/node_modules/<package>/rather than atJSExtensions/<name>/package.json. Phase 4 discovery scans immediate subdirectories for apackage.jsonwith acmdpalsection, so the discovery step may need a follow-up refinement. The gallery-side flow and abstractions here are correct; this is called out for reviewer awareness.Scope
Atomic to Phase 5 (install + uninstall only). Marketplace/auto-update remain out of scope.