Stacked on #288: dependency install for imported pi packages (from #277) - #1
Open
muzimu217 wants to merge 4 commits into
Conversation
Importing a pi CLI extension stripped node_modules and copied only the sources, so any extension with npm dependencies failed to load (jiti reported "Cannot find module ..." and the whole extension was skipped). The generated plugin now keeps the source package.json (plus lockfile) at the plugin root and installs declared dependencies there before the first load: npm install --omit=dev --legacy-peer-deps --no-audit --no-fund --ignore-scripts. Kernel packages keep resolving through virtual modules, and --ignore-scripts means no third-party install script ever runs; a native module that needs one reports its own load error. A failed install never blocks the import — the renderer shows a warning toast with the npm stderr tail. Verified with pi-hermes-memory: the imported plugin loads in the sidecar runtime with its tools, slash commands, and turn hooks registered (previously a load_error).
Pair spec 07-plugins/16 §3.2 with the new import flow (package.json kept at the plugin root, workspaces stripped, pinned npm flags, failure never blocking), add the E2E-PLUGIN-import-extension-* scenarios to the test plan in both locales, spell out the dependency behavior and the Electron-headers rebuild workaround in the plugin development guide, and narrow the README claim to disclose the npm step.
Linux pipes deliver the flood in smaller chunks, so the buffer can sit between the keep size and the 2x trim threshold when the child exits. Assert the actual invariant of the rolling cap (bounded by 2x the keep size after every chunk) instead of the single-chunk macOS result.
Both PRs rewrote the import confirmation in every locale; the combined copy discloses the skills addition and the npm dependency install.
Author
|
更新:vastsa#288 已合并进 main,为免这条叠加线被遗忘,已把同样的提交 rebase 到最新 main 并向上游开了正式 PR: vastsa#300。你这边两条线任选:直接 review/合并上游的 vastsa#300,或先把你 fork 里这个叠加 PR 合进你的分支再由上游处理——都行,内容一致(过滤修复保留你的实现)。辛苦啦~ |
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
1 participant
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.
Stacks the dependency-install half of vastsa#277 on top of this branch, so merging vastsa#288 can carry the whole vastsa#242 scope in one line.
What this adds (3 commits + 1 conflict-resolution):
feat(plugins): install imported pi extension dependencies— when an imported package'spackage.jsondeclaresdependencies, main installs them into the plugin root before first load (npm install --omit=dev --legacy-peer-deps --no-audit --no-fund --ignore-scripts, bounded time, no third-party install scripts, workspaces stripped, lockfile copied alongside). Install status flows to the renderer; a failed install never blocks the import.docs(plugins): …— spec 16 (en + zh-CN) gains the dependency-install paragraph merged with the skills disclosure.test(plugins): …— the stderr-cap regression test, made platform-agnostic.chore(i18n): …— the import confirmation copy now discloses both additions (skills + npm install) in all 8 locales.Conflict resolution notes: your
agent-extensions.tsimplementation of the filter and the manifest hardening supersede ours — I kept yours and dropped ours; only the dependency-install machinery is layered on. Your "not a JSON object" handling already covers what our vastsa#277 review fix did.Validation on the stack:
pnpm build:js, shared 498 tests, agent-extensions suite 7/7, desktop typecheck, biome lint — all green locally.If anything here is out of scope for vastsa#288, cherry-pick what fits or point me at how you'd like it split — happy to rework. / @vastsa this is the "npm 依赖安装" half your scope note deferred to vastsa#277, now rebased onto @Blue-Berrys' branch so one merge line covers vastsa#242 end to end.