feat(skills): discover installed pi CLI skills before import - #814
Merged
Merged
Conversation
Users can find skills installed by pi CLI without locating hidden npm package folders. Keep discovery read-only and require native consent before using the existing trusted import and registration flow. Cover cancellation, stale consent, duplicate imports, runtime skill loading, and the reported published package with isolated tests. fixes vastsa#236
Hoisted npm dependencies and unreadable scope directories must not hide otherwise valid skill packages. Keep per-package validation while letting healthy candidates remain discoverable. An import may be registered before its runtime fails to start. Refresh host-owned state after errors so the panel does not offer a duplicate import, and preserve the error and existing Plugins recovery guidance. Refs vastsa#236
Keep both skill discovery and temporary attachment fork scenarios when merging the current upstream main. Preserve the original feature commits and all upstream behavior without rewriting the shared PR history.
Include the current upstream queue admission fix in the skill discovery candidate so reviewers can validate both changes together. Preserve the original feature implementation and published branch history.
Keep the shared PR current with upstream main while preserving its skill discovery contracts and the contributor's commit history. Validate the combined candidate before updating the existing PR.
Preserve both skill discovery and hosted-search release notes while bringing the shared PR up to current main without rewriting history.
Include the new WebDAV compatibility work before candidate validation. Preserve the original PR behavior and both upstream and author history.
Integrate upstream main at 41367a2 without rewriting the shared PR history. Resolve the ADR index and unreleased notes by preserving entries from both branches. Keep the original pi CLI skill discovery implementation, translations, regression coverage, and trust flow unchanged. Use GitHub's isolated merge preview for non-conflicting files; the scratch preview commit is not part of this branch's history. Full build and runtime test suites were not run in this environment.
Merge upstream main b6c10c6 without rewriting the existing PR history. Preserve both the upstream header-value export and the pi-skill-discovery export. The remaining files merge automatically. The resolved file passed syntax, input-blob, duplicate-export and conflict-marker checks. Full build and runtime tests were not run in this environment.
Preserve both unreleased entries while integrating the current upstream without rewriting the shared PR history. Supply the skill discovery strings for the newly added Brazilian Portuguese locale.
Owner
|
Please route the import flow through the existing “Import Plugin” path rather than adding a separate skill-package import path. Discovery can remain read-only, but importing/enabling should use the existing plugin import lifecycle. |
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
2 participants
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.
Skills installed by pi CLI under
~/.pi/agent/npm/node_modulesare currently invisible unless users locate and import the package directory manually. This follows the explicit discovery and trust flow requested in #236.Settings → Skills now lists installed pi npm packages read-only. Users can inspect the source and declared skills, then explicitly Import and enable through a native confirmation. Cancellation leaves the package untouched; successful imports use the existing trusted-plugin path and are marked Already imported. Discovery never executes package code or silently imports it.
pi-skill-discovery-demo.mp4
The recording shows the original happy path at
c31b63b; the follow-up regression checks below cover the current revision. It uses the publishedplanning-with-files@3.17.1package in an isolated desktop profile and pi CLI home. A local model fixture requests the realSkilltool; its arguments and returned document are shown in the application. No live model service is used.Regression checks cover discovery, cancellation, changed packages during confirmation, duplicate imports, and loading the imported skill in the real runtime. The Skills interaction test also covers retry after failure and refreshing persisted registration after a runtime startup failure. Discovery is verified with 300 hoisted dependencies and an unreadable scope, so unrelated packages cannot hide healthy skills. Build, desktop typecheck, lint, docs checks, and desktop/shared/i18n suites pass.
Scope: globally installed pi npm packages, including scoped package names. This does not scan arbitrary CLI configuration paths, follow symlinked packages, or automatically update imported copies. Imported packages remain manageable in Plugins.
Validation candidate
e7118c9c648c84b696d1d1f902925aa779b941920111e306c120ad5820688d7608cb37bad8fbcc1f7a1309110677661e0aff602619b06ab85dedf5b6; its tree matches the tested task head.pnpm check:pr-base; 33 importer/runtime regression tests including the published package; six React/Chromium interaction scenarios with controlled API results. An isolated real Host and plugin child process additionally verified import, disable, re-enable, process restart, uninstall, and re-import.c31b63b; not a recording of the later failure-path changes.pnpm lint,pnpm docs:check, and the candidate checks above. The initial revision also passedpnpm build:jsand desktop/shared/i18n suites.Fixes #236