feat(plugins): install imported pi extension dependencies

[muzimu217]

Closes #242. Related: #183.

Problem

Importing a pi CLI extension (Plugins → Import pi extension) copies the extension sources without node_modules, and the sidecar loader virtualizes only the kernel packages (pi-ai, pi-agent-core, pi-coding-agent shim, pi-tui stub, typebox). Any bare import of an npm dependency fails at load, the whole extension is reported error, and none of its tools, commands, or hooks register. Reproduced with pi-hermes-memory:

load_error: Cannot find module 'strip-ansi'

Change

• generateImportedExtensionPlugin keeps the source package.json (+ lockfile) at the generated plugin root, stripping any workspaces field so npm never enters the copied sources.

• New installExtensionDependencies runs, when the manifest declares dependencies, before the plugin's first load:

  npm install --omit=dev --legacy-peer-deps --no-audit --no-fund --ignore-scripts
  

  • --legacy-peer-deps keeps pi-coding-agent-style peers out of the tree; disk-installed kernel packages lose to the runtime's virtual modules (verified), so installing them is harmless.
  • --ignore-scripts means no third-party install script ever runs. Modern better-sqlite3 ships prebuilds and works anyway; a native module that needs a build step fails to load with a diagnostic (workaround: npx @electron/rebuild -v <version> in the plugin dir — verified it recovers an ABI-broken case).
  • Bounded time (120 s, SIGTERM → SIGKILL escalation), rolling stderr cap, injectable runner for tests.

• A failed install never blocks the import: the renderer shows a warning toast with the npm stderr tail, the plugin registers, and the extension reports its own load_error diagnostic.

• The import confirm dialog now discloses the npm step (network egress; scripts disabled).

Security notes

• The npm step is disclosed in the existing confirm (the trust decision) and pinned to literal, non-interpolated flags; no shell on POSIX.
• --ignore-scripts removes third-party install-script execution from the import path entirely.
• workspaces stripping prevents npm from treating the copied source tree as a workspace.
• agent.extension gating and the diagnostics surface are unchanged.

Validation

• Gates: pnpm build:js, desktop typecheck, pnpm lint, pnpm -r test (1455 passing), pnpm docs:check (77 en/zh pairs) — all green.

• E2E: pnpm test:e2e → 18/18 passed (2 skipped: live-model suites requiring PI_DESKTOP_TEST_API_KEY, unrelated to this surface).

• New unit tests cover: package.json/lockfile placement, workspaces stripping, never copying node_modules, pinned npm flags and cwd, failure/skip/invalid-manifest paths, stderr cap, and timeout kill.

• New E2E scenario docs (en/zh paired): E2E-PLUGIN-import-extension-installs-dependencies, E2E-PLUGIN-import-extension-reports-missing-dependency.

• End-to-end proof with pi-hermes-memory (fresh clone, no node_modules) imported through the new flow and loaded through the same esbuild sidecar bundle as E2E-245, under both system Node and the Electron runtime:

  state: loaded
  tools:    6  (memory_add, memory_replace, memory_remove, skill_manage, session_search, memory_search)
  commands: 10 (memory-consolidate, memory-insights, memory-interview, …)
  hooks:    8  (session_start, resources_discover, before_agent_start, tool_result,
              message_end, turn_end, session_before_compact, session_shutdown)
  diagnostics: 0
  

Spec & docs

• 07-plugins/16-trusted-extensions.md §3.2 (en/zh): the import flow's dependency semantics.
• 06-delivery/04-e2e-test-plan.md (en/zh): two new scenarios.
• docs/plugin-development.md §6.11 (en/zh): dependency behavior + native-module workaround.
• README / README.zh-CN: the pi-extensions section now discloses the npm step.

Known limits / follow-ups

1. ctx.sessionManager is a v1 stub (spec §5, deferred to v2), so hermes-style session indexing stays limited — that part needs the v2 read shim.
2. While testing native modules we noticed a dlopen failure inside an extension escapes the Runner's load guard as an uncaughtException (sidecar crash) instead of a clean load_error diagnostic; happy to follow up with a small hardening PR.
3. Native modules that need build scripts remain diagnostic-only by design until a rebuild story is agreed.

[vastsa]

Request changes: the direction is sound, but I found two correctness issues that should be fixed before merge.

1. Blocking import failure for the exact npm-installed path from issue #242. In apps/desktop/electron/main/agent-extensions.ts:399, the directory copy filters with p.includes("node_modules"). When the selected source itself is under ~/.pi/agent/npm/node_modules/<package> (the path described in #242), the filter rejects the source root, so no extension files are copied. The generated manifest still points at src/..., but the entry is missing and the extension does not register. I reproduced this with a temporary node_modules/pi-maestro-flow source: copiedEntry: false, while the generated package.json was present. Filter only descendant node_modules segments relative to the selected root, and add a regression test whose source directory is itself nested under node_modules.

2. Valid JSON can still abort the import. In apps/desktop/electron/main/agent-extensions.ts:336, JSON.parse() is caught, but a valid package.json containing null makes manifest.dependencies throw a TypeError. Because IPC awaits this at agent-extensions-ipc.ts:80 before loadDevPlugin(), the documented “failed install never blocks the import” behavior is violated. Validate that the parsed manifest is a non-null object (or keep the access inside the guarded path) and add a regression test.

The current required GitHub checks are green for JS and Rust; the Vercel status is red only because deployment authorization is required. I did not run E2E locally because this review request did not authorize it and the repository checklist requires an explicit request. The PR description’s reported E2E result therefore remains independently unverified here.

There is also a documentation completeness follow-up: the two new E2E scenario IDs are not added to the traceability matrix in docs/spec/06-delivery/04-e2e-test-plan.md.

[muzimu217]

Thanks for the fast, precise review — both findings reproduced locally before fixing, exactly as described.

1. npm-layout source (#242) copied nothing — reproduced (copiedEntry: false with a source under a fake node_modules tree): the absolute-path includes("node_modules") filter rejected the selected root itself. Fixed in af098fe: the filter now excludes only node_modules segments below the selected root (relative(resolved, p).split(/[\\/]/).includes("node_modules")).

Regression test added (importing a source directory that itself sits under node_modules keeps its entries): asserts the declared entry is copied, package.json is kept, descendant files are copied, and descendant node_modules is still excluded.

End-to-end proof through the full new flow with the exact #242 shape (source selected from inside a …/npm/node_modules/pi-flow tree, dependencies declared): generate → entry copied ✅ → install {"state":"installed"} ✅ → loaded through the same esbuild sidecar bundle as E2E-245: state: "loaded", tool flow_probe + hook before_agent_start registered, 0 diagnostics.

2. Valid-JSON null aborted the import — reproduced (Cannot read properties of null (reading 'dependencies') thrown after the parse try/catch). Fixed: any non-object parse result (null, arrays) now returns { state: "failed", error: "package.json is not a JSON object" }, so the import never blocks. Regression tests added for both null and [].

3. Traceability matrix — both new scenario IDs added to the "Trusted extensions (R7 v1)" row in en and zh (docs:check still verifies 77 pairs).

Local validation on af098fe: pnpm build:js, desktop typecheck, pnpm lint, pnpm docs:check, pnpm -r test (1455 passing; agent-extensions suite 8/8, run 3×), pnpm test:e2e 18/18 (2 live-model skips). Understood on E2E — if you'd like, say the word and I'll request the E2E harness run, or treat the checklist however you prefer.

[muzimu217]

Superseded by #288 — @Blue-Berrys covers the node_modules filter fix and extends further into skill import, so keeping one PR is cleaner.

Our unique part here (the dependencies-install flow for imported extensions, plus the malformed-manifest handling and its regression tests) has been offered over on #288 for whoever wants to fold it in; the branch stays available at af098fe if it's useful.

Thanks for the precise review @vastsa — closing this to keep the queue tidy. (=^･ω･^=)