Skip to content

fix(plugins): 修复已安装 pi npm 包的技能导入 - #288

Merged
vastsa merged 1 commit into
vastsa:mainfrom
Blue-Berrys:fix/issue-236-package-skills
Sep 13, 2026
Merged

fix(plugins): 修复已安装 pi npm 包的技能导入#288
vastsa merged 1 commit into
vastsa:mainfrom
Blue-Berrys:fix/issue-236-package-skills

Conversation

@Blue-Berrys

Copy link
Copy Markdown
Contributor

修复 #236 中已安装 npm 包手动导入后仍不显示技能的问题。选择 ~/.pi/agent/npm/node_modules/planning-with-files 包目录时,原复制过滤器会因为祖先路径包含 node_modules 而丢弃整个包;此外导入器只映射 pi.extensions,遗漏了 pi.skills

此改动按包内相对路径排除依赖,保留技能的脚本、模板与资源,将显式声明的技能文件/目录映射为插件技能,并为同名 SKILL.md 生成独立稳定 ID。仅包含技能的包只声明 agent.prompt.inject,不会把辅助 index.js 当作可执行扩展。补齐路径校验、权限撤销/卸载回归、中英文规范及导入确认文案。

验证(macOS ARM64,Node 25.5.0):

  • 先复现两项导入失败,再通过修复;桌面完整测试 1,473/1,473 通过,包含 20 项导入/运行时回归。
  • pnpm build:js、desktop typecheck、pnpm lint、77 对规格本地化检查通过。
  • pnpm test:e2e:18/18 通过;两项真实模型测试缺少 PI_DESKTOP_TEST_API_KEY 跳过。pnpm test:e2e:boot 通过。
  • 真实 planning-with-files@3.17.1 经同一导入器及真实 PluginRuntime 子进程加载:44 个文件完整保留,pi-planning-with-files 进入目录,34,316-byte 正文与原文解析结果一致;权限撤销、恢复及卸载通过。未执行第三方 extension、技能脚本或模型调用。
  • 额外双技能样本经真实 Host 安装、查询、读取和卸载通过。

范围:这是显式本地包导入,不自动扫描 ~/.pi,不实现完整 pi CLI 包管理。npm 依赖安装仍由 #277 处理。原生选择器完整 UI 操作和真实模型调用未验证;不声称完成所有 npm 扩展兼容或整个 #242

Refs #236

@muzimu217

Copy link
Copy Markdown
Contributor

👍 这条 PR 和我们昨天提的 #277 修的是同一个 node_modules 过滤问题,而且技能映射(pi.skills、稳定 ID、skill-only 包)做得比我们更完整——建议以这条为主线。

为了不留下两个重叠的 PR,我们打算把 #277 关掉。#277 里有一点可能对这条 PR 有用的补充,如果需要的话我们随时提供补丁(分支 feat/import-extension-deps @ af098fe,MIT 同源):

  1. 依赖安装流程(PR feat(plugins): install imported pi extension dependencies #277 的主体,fix(plugins): 修复已安装 pi npm 包的技能导入 #288 目前未覆盖):导入的扩展声明 dependencies 时,导入后自动安装依赖,安装状态回传 UI({"state":"installed"}),端到端验证过经 esbuild sidecar 加载、工具与 hook 正常注册;
  2. 畸形 manifest 的容错:package.json 解析结果为 null / 数组等非对象时,原实现会在 try/catch 之外抛错中断导入——已改为返回 { state: "failed", error: "package.json is not a JSON object" } + 回归测试;
  3. 一组 agent-extensions 回归测试(node_modules 根目录选择、依赖排除、卸载/权限撤销)。

如果这些都已在你的实现里覆盖,那就最好,关掉 #277 即可;如果有缺的,我们可以开一个指向你分支的 PR 把补丁送过来,或者你 cherry-pick 也行。怎么方便怎么来~

@muzimu217

Copy link
Copy Markdown
Contributor

跟进:为方便整合,把 #277 的依赖安装部分 rebase 到了你的分支之上,开了一个叠加 PR:Blue-Berrys#1(只含依赖安装 + 文档 + 测试,过滤修复保留你的实现)。合并顺序 = 先 #288 再这个,#242 就能一条线闭环。冲突解决说明和验证结果都写在叠加 PR 描述里,需要调整随时说~

@vastsa vastsa left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已审查 0d966258 与当前主线 76cd2dbd 的集成结果,未发现阻断问题。导入目录边界、符号链接拒绝、技能数量限制、纯技能包权限及禁用/卸载路径均保留了现有约束。

独立验证的集成 tree:bb2a239273ce99eb5320d93101cc0c7c172a4d35

  • pnpm build:js 通过。
  • 三个 importer/runtime 回归测试文件:20/20 通过。
  • pnpm test:e2e:18 项通过;2 项真实模型/流式场景因未配置测试密钥跳过,不计为通过。
  • 原 PR 的 JS、Rust、文档 CI 均通过。Vercel 的 fork 部署授权状态不属于产品测试失败。

验证使用独立 worktree 和临时 profile,保留原作者提交。批准合并。

@vastsa
vastsa merged commit d285274 into vastsa:main Sep 13, 2026
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants