feat(plugins): install dependencies for imported pi packages (follow-up to #288) - #300
Conversation
|
审查结论：暂不合并（BLOCK）。当前 head 为 P1：npm 安装未建立明确的环境、网络和凭据边界
P1：
合入门禁 PR 描述中的 非阻断但建议一并处理：安装失败后的部分 |
|
收到,两个 P1 都确认属实,修复计划: 1. npm 安装的环境/网络/凭据边界 — 确认:
2. null manifest TypeError — 确认:af098fe 的非空对象校验没被带进当前 head(我在叠栈时误判 #288 已覆盖,实际它只覆盖了导入路径;#300 新增的安装路径自己 parse 的那次没有 guard)。修复:非对象解析结果返回 合入门禁 — 确认 18/18 引用不当;修复后以当前 head 重跑 非阻断四项一并处理:失败安装的 node_modules/lockfile 尽力清理、spawn 加 windowsHide、lockfile 与可复现性关系在文档中说明、spec16 npm 表述对齐。修完重跑 CI + E2E 后请求复审。 |
Per review on vastsa#300: npm now runs with an explicit minimal environment (PATH/HOME/TMPDIR/LANG plus forced ignore-scripts/audit/fund flags) so npm tokens, proxy and SSH configuration cannot leak into the install. Dependency specs are registry-only — file:/link:/git/http(s) specs are rejected before npm runs — and a copied lockfile is kept only when every resolved URL points at registry.npmjs.org, otherwise dropped so npm resolves from package.json. The installer also windows-hides the child. JSON.parse results of null or an array return a failed state instead of throwing past the guard; regression tests cover null, [], non-registry specs, and non-registry lockfile drops.
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.
Per review on vastsa#300: npm now runs with an explicit minimal environment (PATH/HOME/TMPDIR/LANG plus forced ignore-scripts/audit/fund flags) so npm tokens, proxy and SSH configuration cannot leak into the install. Dependency specs are registry-only — file:/link:/git/http(s) specs are rejected before npm runs — and a copied lockfile is kept only when every resolved URL points at registry.npmjs.org, otherwise dropped so npm resolves from package.json. The installer also windows-hides the child. JSON.parse results of null or an array return a failed state instead of throwing past the guard; regression tests cover null, [], non-registry specs, and non-registry lockfile drops.
1ce6de9 to
5236f05
Compare
# Conflicts: # docs/spec/06-delivery/04-e2e-test-plan.md # docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md
|
Merged as
Validation: local |
|
Merged with the landing fixes — thank you, @vastsa! The npm environment isolation, registry pinning, and lockfile validation hardening make the install path substantially safer. Appreciated the fast turnaround on this one. |
2 participants
Follow-up to #288 (merged) — delivers the half its scope note deferred: "npm 依赖安装仍由 #277 处理". Closes #242's remaining scope. Supersedes the dependency-install half of the closed #277; the import-filter fix and manifest hardening live in #288 and are kept as-is.
What it adds
When an imported package's
package.jsondeclaresdependencies, main installs them into the plugin root before the first load:npm install --omit=dev --legacy-peer-deps --no-audit --no-fund --ignore-scripts— bounded time, no third-party install script ever runs, kernel packages keep resolving through virtual modulesworkspacesfield strippedValidation
pnpm build:js, shared 498 tests, agent-extensions suite 7/7, desktop 1,498 tests, typecheck, biome lint, docs:check (77 pairs) — all green…/npm/node_modules/<pkg>,dependenciesdeclared), carried over from feat(plugins): install imported pi extension dependencies #277's validation: generate → entry copied → install{"state":"installed"}→ loaded through the esbuild sidecar, tool + hook registered, 0 diagnosticspnpm test:e2e18/18 passed, 2 live-model cases skipped without a key (E2E-008/009) — rerun after the boundary fixes belowReview fixes (2026-09-13, per the BLOCK review)
spawn("npm")改为显式最小环境(PATH/HOME/TMPDIR/LANG + 强制 ignore-scripts/audit/fund/update-notifier),npm token、代理、SSH 配置不再泄漏进安装file:/link:/git*/http(s)在 npm 启动前拒绝并报错(回归测试断言 npm 未被调用)resolved不是https://registry.npmjs.org/即丢弃 lockfile 改由 registry 解析;yarn/pnpm lockfile 直接不复制JSON.parse("null")/[]返回{"state":"failed","error":"package.json is not a JSON object"},不再抛穿;回归测试覆盖windowsHide: true;lockfile 丢弃与可复现性的取舍已在注释中说明