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

apps/desktop/electron/main/agent-extensions.ts

@@ -8,9 +8,10 @@
  * the modal prompts between the sidecar and the renderer. Discovery and
  * enablement are the plugin system's job; nothing here touches the filesystem.
  */
-import { randomUUID } from "node:crypto";
-import { cpSync, existsSync, mkdirSync, statSync, writeFileSync } from "node:fs";
-import { basename, extname, join, relative, resolve } from "node:path";
+import { createHash, randomUUID } from "node:crypto";
+import { cpSync, existsSync, lstatSync, mkdirSync, realpathSync, rmSync, statSync, writeFileSync } from "node:fs";
+import { basename, dirname, extname, isAbsolute, join, relative, resolve, sep } from "node:path";
+import { assertImportedPackagePath, discoverImportedPackageSkills } from "./imported-package-skills";
 import { discoverManualPath } from "@pi-desktop/agent-runtime";
 import {
   ErrorCodes,
@@ -255,25 +256,51 @@ export function generateImportedExtensionPlugin(
   source: string,
   importRoot: string,
 ): { path: string; id: string; entries: string[] } {
-  const resolved = resolve(source);
-  const specs = discoverManualPath(resolved);
-  if (specs.length === 0) {
+  const resolved = existsSync(source) ? realpathSync(source) : resolve(source);
+  const isDirectory = existsSync(resolved) && statSync(resolved).isDirectory();
+  const { skills, skillsOnly } = isDirectory
+    ? discoverImportedPackageSkills(resolved)
+    : { skills: [], skillsOnly: false };
+  // A skill-only package may contain helper scripts; do not promote a root
+  // index.js or other incidental script to executable agent extensions.
+  const specs = skillsOnly ? [] : discoverManualPath(resolved);
+  if (specs.length === 0 && skills.length === 0) {
     throw Object.assign(new Error("no extension entry found at that path"), {
       errorCode: ErrorCodes.INVALID_ARGUMENT,
     });
   }
-  const isDirectory = statSync(resolved).isDirectory();
+  for (const spec of specs) assertImportedPackagePath(isDirectory ? resolved : dirname(resolved), spec.entry);
   const slug = slugFor(resolved);
   const id = `${PLUGIN_ID_PREFIX}${slug}`;
   let dir = join(importRoot, slug);
   let suffix = 2;
   while (existsSync(dir)) dir = join(importRoot, `${slug}-${suffix++}`);
+  const destinationRelative = relative(resolved, resolve(dir));
+  if (isDirectory && !isAbsolute(destinationRelative) && !destinationRelative.split(sep).includes("..")) {
+    throw Object.assign(new Error("import destination cannot be inside the selected package"), {
+      errorCode: ErrorCodes.INVALID_ARGUMENT,
+    });
+  }
   const srcDir = join(dir, "src");
-  mkdirSync(srcDir, { recursive: true });
-  if (isDirectory) {
-    cpSync(resolved, srcDir, { recursive: true, filter: (p) => !p.includes("node_modules") });
-  } else {
-    cpSync(resolved, join(srcDir, basename(resolved)));
+  try {
+    mkdirSync(srcDir, { recursive: true });
+    if (isDirectory) {
+      cpSync(resolved, srcDir, { recursive: true, filter: (path) => {
+        // Exclude dependencies within the selection, not its npm installation ancestors.
+        if (relative(resolved, path).split(sep).includes("node_modules")) return false;
+        if (lstatSync(path).isSymbolicLink()) {
+          throw Object.assign(new Error("imported packages cannot contain a symbolic link"), {
+            errorCode: ErrorCodes.INVALID_ARGUMENT,
+          });
+        }
+        return true;
+      } });
+    } else {
+      cpSync(resolved, join(srcDir, basename(resolved)));
+    }
+  } catch (error) {
+    rmSync(dir, { recursive: true, force: true });
+    throw error;
   }
   const entries = specs.map((spec) =>
     isDirectory
@@ -287,13 +314,20 @@ export function generateImportedExtensionPlugin(
     version: "0.0.0",
     description: `Imported pi extension from ${resolved}`,
     main: "main.js",
-    permissions: ["agent.extension"],
-    contributes: { agentExtensions: entries },
+    permissions: [...(entries.length ? ["agent.extension"] : []), ...(skills.length ? ["agent.prompt.inject"] : [])],
+    contributes: {
+      ...(entries.length ? { agentExtensions: entries } : {}),
+      ...(skills.length ? { skills: skills.map((path) => ({
+        path: `src/${path}`,
+        // Distinct directories often use the same SKILL.md basename.
+        id: `skill-${createHash("sha256").update(path).digest("hex").slice(0, 16)}`,
+      })) } : {}),
+    },
   };
   writeFileSync(join(dir, "manifest.json"), JSON.stringify(manifest, null, 2) + "\n", "utf8");
   writeFileSync(
     join(dir, "main.js"),
-    "// Generated by PI-Desktop: this plugin only contributes agent extensions.\nmodule.exports = {};\n",
+    "// Generated by PI-Desktop: declarative skills and/or agent extensions.\nmodule.exports = {};\n",
     "utf8",
   );
   return { path: dir, id, entries };

apps/desktop/electron/main/imported-package-skills.ts

@@ -0,0 +1,90 @@
+import { existsSync, lstatSync, readdirSync, readFileSync } from "node:fs";
+import { isAbsolute, join, relative, resolve, sep, win32 } from "node:path";
+import { ErrorCodes } from "@pi-desktop/shared";
+
+function invalid(message: string): never {
+  throw Object.assign(new Error(message), { errorCode: ErrorCodes.INVALID_ARGUMENT });
+}
+
+/** Validate before reading a declared contribution; do not follow child links. */
+export function assertImportedPackagePath(root: string, target: string): void {
+  const rel = relative(root, target);
+  const parts = rel.split(sep).filter(Boolean);
+  if (isAbsolute(rel) || parts.includes("..") || parts.includes("node_modules")) {
+    invalid("imported contribution must stay inside the selected package, outside node_modules");
+  }
+  let current = root;
+  for (const part of parts) {
+    current = join(current, part);
+    if (lstatSync(current).isSymbolicLink()) {
+      invalid("imported contributions cannot contain a symbolic link");
+    }
+  }
+}
+
+/** Map explicit pi.skills files/directories to existing plugin contributions. */
+export function discoverImportedPackageSkills(root: string): { skills: string[]; skillsOnly: boolean } {
+  const packagePath = join(root, "package.json");
+  if (!existsSync(packagePath)) return { skills: [], skillsOnly: false };
+  assertImportedPackagePath(root, packagePath);
+  let metadata: { pi?: { skills?: unknown; extensions?: unknown } };
+  try {
+    metadata = JSON.parse(readFileSync(packagePath, "utf8"));
+  } catch (error) {
+    // Keep the existing loose-extension fallback for malformed package metadata.
+    if (error instanceof SyntaxError) return { skills: [], skillsOnly: false };
+    throw error;
+  }
+  const declared = metadata?.pi?.skills;
+  if (declared === undefined) return { skills: [], skillsOnly: false };
+  if (!Array.isArray(declared) || declared.length > 32) {
+    invalid("pi.skills must be an array of at most 32 relative files or directories");
+  }
+  const extensions = metadata.pi?.extensions;
+  if (extensions !== undefined && !Array.isArray(extensions)) {
+    invalid("pi.extensions must be an array when importing a skill package");
+  }
+  const skills = new Set<string>();
+  const visited = new Set<string>();
+  const add = (path: string) => {
+    skills.add(relative(root, path).split(sep).join("/"));
+    if (skills.size > 32) invalid("a package can import at most 32 skills");
+  };
+  const scan = (dir: string, rootFiles: boolean) => {
+    const scanKey = `${rootFiles}:${dir}`;
+    if (visited.has(scanKey)) return;
+    visited.add(scanKey);
+    if (visited.size > 256) invalid("pi.skills directory scan exceeds 256 directories");
+    const ownSkill = join(dir, "SKILL.md");
+    if (existsSync(ownSkill)) {
+      assertImportedPackagePath(root, ownSkill);
+      if (lstatSync(ownSkill).isFile()) {
+        add(ownSkill);
+        return;
+      }
+    }
+    for (const entry of readdirSync(dir, { withFileTypes: true }).sort((a, b) => a.name.localeCompare(b.name))) {
+      if (entry.name.startsWith(".") || entry.name === "node_modules") continue;
+      const path = join(dir, entry.name);
+      assertImportedPackagePath(root, path);
+      if (entry.isDirectory()) scan(path, false);
+      else if (entry.isFile() && rootFiles && entry.name.endsWith(".md")) add(path);
+    }
+  };
+  for (const entry of declared) {
+    if (typeof entry !== "string" || !entry.trim()) invalid("pi.skills entries must be nonempty paths");
+    if (isAbsolute(entry) || win32.isAbsolute(entry) || entry.split(/[\\/]/).includes("..")) {
+      invalid("pi.skills paths must be relative to the selected package");
+    }
+    const path = resolve(root, entry.replaceAll("\\", "/"));
+    // Diagnose unavailable or unsupported declarations rather than silently
+    // importing a package which teaches the agent none of its declared skills.
+    if (!existsSync(path)) invalid(`pi.skills path does not exist: ${entry}`);
+    assertImportedPackagePath(root, path);
+    const info = lstatSync(path);
+    if (info.isDirectory()) scan(path, true);
+    else if (info.isFile() && path.endsWith(".md")) add(path);
+    else invalid(`pi.skills path must be a Markdown file or directory: ${entry}`);
+  }
+  return { skills: [...skills].sort(), skillsOnly: extensions === undefined || extensions.length === 0 };
+}

apps/desktop/test/agent-extensions.test.mjs

@@ -1,13 +1,15 @@
 import assert from "node:assert/strict";
 import test from "node:test";
+import { register } from "node:module";
 import { existsSync, mkdirSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 
-import {
+register(new URL("./helpers/ts-import-hooks.mjs", import.meta.url));
+const {
   AgentExtensionBridge,
   generateImportedExtensionPlugin,
-} from "../electron/main/agent-extensions.ts";
+} = await import("../electron/main/agent-extensions.ts");
 
 function bridge(overrides = {}) {
   const events = { changed: 0, prompts: [], toasts: [], statuses: [] };

apps/desktop/test/imported-package-skills-runtime.test.mjs

@@ -0,0 +1,154 @@
+import assert from "node:assert/strict";
+import { fork } from "node:child_process";
+import {
+  existsSync,
+  mkdirSync,
+  mkdtempSync,
+  readFileSync,
+  realpathSync,
+  rmSync,
+  writeFileSync,
+} from "node:fs";
+import { register } from "node:module";
+import { tmpdir } from "node:os";
+import { dirname, join } from "node:path";
+import test from "node:test";
+import { fileURLToPath } from "node:url";
+
+register(new URL("./helpers/ts-import-hooks.mjs", import.meta.url));
+const { generateImportedExtensionPlugin } = await import("../electron/main/agent-extensions.ts");
+const { PluginRuntime } = await import("../electron/main/plugin-runtime.ts");
+const hostEntry = fileURLToPath(new URL("../electron/main/plugin-host-process.mjs", import.meta.url));
+
+function createHarness(t, { skillsOnly = false } = {}) {
+  const root = mkdtempSync(join(tmpdir(), "pi-imported-package-skills-"));
+  // The selected package itself lives under node_modules, as an npm global
+  // installation would. Its own nested dependency tree must still be excluded.
+  const source = join(root, "npm", "node_modules", "@fixture", "package-skills");
+  const importRoot = join(root, "imported");
+  const write = (path, content) => {
+    const absolute = join(source, path);
+    mkdirSync(dirname(absolute), { recursive: true });
+    writeFileSync(absolute, content, "utf8");
+  };
+  const bodies = new Map([
+    ["direct", "# Direct fixture\n\nUse fixture input."],
+    ["release", "# Release fixture\n\nRead references/guide.txt before using assets/input.json."],
+    ["alpha", "# Alpha fixture\n\nInspect fixture alpha."],
+    ["beta", "# Beta fixture\n\nInspect fixture beta."],
+  ]);
+  const paths = new Map([
+    ["direct", "skills/direct.md"],
+    ["release", "skills/release/SKILL.md"],
+    ["alpha", "skills/catalog/alpha/SKILL.md"],
+    ["beta", "skills/catalog/beta/SKILL.md"],
+  ]);
+  write("package.json", JSON.stringify({
+    name: "@fixture/package-skills",
+    version: "1.0.0",
+    pi: {
+      ...(!skillsOnly ? { extensions: ["index.ts"] } : {}),
+      skills: ["skills/direct.md", "skills/release", "skills/catalog"],
+    },
+  }));
+  if (!skillsOnly) write("index.ts", "export default function () {}\n");
+  for (const [name, path] of paths) {
+    write(path, `---\nname: ${name}\ndescription: Test ${name} skill.\n---\n\n${bodies.get(name)}\n`);
+  }
+  write("skills/release/references/guide.txt", "Fixture reference contents.\n");
+  write("skills/release/assets/input.json", '{"fixture":true}\n');
+  write("lib/node_modules-note.txt", "This is a resource, not a dependency directory.\n");
+  write("node_modules/fixture-dependency/index.js", "module.exports = {};\n");
+
+  const audits = [];
+  const runtime = new PluginRuntime({
+    hostEntry,
+    spawnProcess: ({ entry }) => {
+      // Execute only the repository's real plugin host and the importer's
+      // generated no-op main.js. Fixture extension modules are only catalogued.
+      const child = fork(entry, [], { stdio: ["ignore", "pipe", "pipe", "ipc"] });
+      return {
+        postMessage: (message) => { if (child.connected) child.send(message); },
+        onMessage: (handler) => child.on("message", handler),
+        onExit: (handler) => child.on("exit", (code) => handler(code ?? 0)),
+        kill: () => child.kill(),
+      };
+    },
+    audit: (entry) => audits.push(entry),
+  });
+  t.after(async () => {
+    for (const loaded of runtime.listLoaded()) await runtime.unload(loaded.manifest.id);
+    runtime.disposeWatchers();
+    rmSync(root, { recursive: true, force: true });
+  });
+  return { source, importRoot, runtime, audits, bodies, paths };
+}
+
+function assertSkillCatalog(runtime, imported, bodies, paths) {
+  const catalog = runtime.getSkills();
+  assert.deepEqual(catalog.map((skill) => skill.name).sort(), [...bodies.keys()].sort());
+  assert.equal(new Set(catalog.map((skill) => skill.id)).size, bodies.size,
+    "different SKILL.md directories must not collide on one basename-derived ID");
+  for (const skill of catalog) {
+    assert.equal(skill.pluginId, imported.id);
+    assert.ok(skill.id.startsWith(`${imported.id}/`));
+    assert.equal(realpathSync(skill.path), realpathSync(join(imported.path, "src", paths.get(skill.name))));
+    assert.equal(skill.description, `Test ${skill.name} skill.`);
+    assert.deepEqual(runtime.loadSkillBody(skill.id), {
+      id: skill.id, name: skill.name, body: bodies.get(skill.name),
+    });
+  }
+  return catalog;
+}
+
+test("an imported npm package exposes all declared skill files and directories through the real runtime", async (t) => {
+  const { source, importRoot, runtime, bodies, paths } = createHarness(t);
+  const imported = generateImportedExtensionPlugin(source, importRoot);
+  await runtime.loadFromPath(imported.path);
+  const catalog = assertSkillCatalog(runtime, imported, bodies, paths);
+  assert.equal(runtime.getAgentExtensions().length, 1);
+  assert.equal(realpathSync(runtime.getAgentExtensions()[0].entry), realpathSync(join(imported.path, "src/index.ts")));
+  assert.equal(readFileSync(join(imported.path, "src/skills/release/references/guide.txt"), "utf8"), "Fixture reference contents.\n");
+  assert.deepEqual(JSON.parse(readFileSync(join(imported.path, "src/skills/release/assets/input.json"), "utf8")), { fixture: true });
+  assert.ok(existsSync(join(imported.path, "src/lib/node_modules-note.txt")));
+  assert.equal(existsSync(join(imported.path, "src/node_modules")), false);
+
+  await runtime.unload(imported.id);
+  assert.deepEqual(runtime.getSkills(), []);
+  assert.deepEqual(runtime.getAgentExtensions(), []);
+  for (const skill of catalog) {
+    assert.throws(() => runtime.loadSkillBody(skill.id), (error) => error.code === "NOT_FOUND");
+  }
+});
+
+test("imported skill grants remain independent of agent extension grants and are revoked on reload", async (t) => {
+  const { source, importRoot, runtime, audits, bodies, paths } = createHarness(t);
+  const imported = generateImportedExtensionPlugin(source, importRoot);
+  await runtime.loadFromPath(imported.path);
+  const catalog = assertSkillCatalog(runtime, imported, bodies, paths);
+
+  await runtime.loadFromPath(imported.path, ["agent.extension"]);
+  assert.equal(runtime.getAgentExtensions().length, 1);
+  assert.deepEqual(runtime.getSkills(), []);
+  assert.ok(audits.some((entry) => entry.pluginId === imported.id &&
+    entry.api === "plugin.skills.skipped" && entry.errorCode === "PERMISSION_DENIED"));
+  for (const skill of catalog) {
+    assert.throws(() => runtime.loadSkillBody(skill.id), (error) => error.code === "NOT_FOUND");
+  }
+
+  await runtime.loadFromPath(imported.path, ["agent.prompt.inject"]);
+  assert.deepEqual(runtime.getAgentExtensions(), []);
+  const restored = assertSkillCatalog(runtime, imported, bodies, paths);
+  assert.deepEqual(restored.map((skill) => skill.id), catalog.map((skill) => skill.id));
+});
+
+test("a skill-only pi package loads in the plugin runtime without requiring executable extensions", async (t) => {
+  const { source, importRoot, runtime, bodies, paths } = createHarness(t, { skillsOnly: true });
+  const imported = generateImportedExtensionPlugin(source, importRoot);
+  await runtime.loadFromPath(imported.path);
+  assertSkillCatalog(runtime, imported, bodies, paths);
+  assert.deepEqual(runtime.getAgentExtensions(), []);
+  const manifest = runtime.getLoaded(imported.id).manifest;
+  assert.ok(manifest.permissions.includes("agent.prompt.inject"));
+  assert.equal(manifest.permissions.includes("agent.extension"), false);
+});