From 0227c8c7f1e258c3ab296ad90401eea87f800101 Mon Sep 17 00:00:00 2001 From: vastsa Date: Fri, 18 Sep 2026 21:31:38 +0800 Subject: [PATCH] fix(plugins): migrate generated imported main.js wrappers on load Existing imported ESM packages still fail after the main.cjs generator fix. Rewrite the generated no-op in place when the plugin loads, and leave customized main.js and copied package files untouched. --- .../desktop/electron/main/agent-extensions.ts | 14 ++++-- .../electron/main/imported-plugin-wrapper.ts | 50 +++++++++++++++++++ apps/desktop/electron/main/plugin-runtime.ts | 3 ++ apps/desktop/test/agent-extensions.test.mjs | 47 +++++++++++++++++ .../imported-package-skills-runtime.test.mjs | 33 ++++++++++++ docs/spec/06-delivery/04-e2e-test-plan.md | 9 ++-- docs/spec/07-plugins/16-trusted-extensions.md | 12 +++-- .../spec/06-delivery/04-e2e-test-plan.md | 6 ++- .../spec/07-plugins/16-trusted-extensions.md | 8 +-- 9 files changed, 164 insertions(+), 18 deletions(-) create mode 100644 apps/desktop/electron/main/imported-plugin-wrapper.ts diff --git a/apps/desktop/electron/main/agent-extensions.ts b/apps/desktop/electron/main/agent-extensions.ts index 838cdb683..39920f598 100644 --- a/apps/desktop/electron/main/agent-extensions.ts +++ b/apps/desktop/electron/main/agent-extensions.ts @@ -14,6 +14,11 @@ import { createHash, randomUUID } from "node:crypto"; import { copyFileSync, cpSync, existsSync, lstatSync, mkdirSync, readFileSync, 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 { + IMPORTED_PLUGIN_ID_PREFIX, + IMPORTED_PLUGIN_MAIN, + IMPORTED_PLUGIN_WRAPPER_SOURCE, +} from "./imported-plugin-wrapper"; import { discoverManualPath } from "@pi-desktop/agent-runtime"; export { defaultDependencyRunner, installExtensionDependencies } from "./npm-installer"; export type { DependencyCommandRunner, ExtensionDependencyInstallResult } from "./npm-installer"; @@ -244,7 +249,6 @@ export class AgentExtensionBridge { } -const PLUGIN_ID_PREFIX = "imported."; const NPM_LOCKFILE_NAMES = ["package-lock.json", "npm-shrinkwrap.json"] as const; const IMPORT_SENSITIVE_FILE_NAMES = new Set([ @@ -341,7 +345,7 @@ export function generateImportedExtensionPlugin( throw error; } } - const id = `${PLUGIN_ID_PREFIX}${basename(dir)}`; + const id = `${IMPORTED_PLUGIN_ID_PREFIX}${basename(dir)}`; const srcDir = join(dir, "src"); try { mkdirSync(srcDir); @@ -373,7 +377,7 @@ export function generateImportedExtensionPlugin( name: slug, version: "0.0.0", description: `Imported pi extension from ${resolved}`, - main: "main.cjs", + main: IMPORTED_PLUGIN_MAIN, permissions: [...(entries.length ? ["agent.extension"] : []), ...(skills.length ? ["agent.prompt.inject"] : [])], contributes: { ...(entries.length ? { agentExtensions: entries } : {}), @@ -386,8 +390,8 @@ export function generateImportedExtensionPlugin( }; writeFileSync(join(dir, "manifest.json"), JSON.stringify(manifest, null, 2) + "\n", "utf8"); writeFileSync( - join(dir, "main.cjs"), - "// Generated by PI-Desktop: declarative skills and/or agent extensions.\nmodule.exports = {};\n", + join(dir, IMPORTED_PLUGIN_MAIN), + IMPORTED_PLUGIN_WRAPPER_SOURCE, "utf8", ); if (isDirectory) { diff --git a/apps/desktop/electron/main/imported-plugin-wrapper.ts b/apps/desktop/electron/main/imported-plugin-wrapper.ts new file mode 100644 index 000000000..da82ccf33 --- /dev/null +++ b/apps/desktop/electron/main/imported-plugin-wrapper.ts @@ -0,0 +1,50 @@ +import { existsSync, readFileSync, unlinkSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; + +export const IMPORTED_PLUGIN_ID_PREFIX = "imported."; +export const IMPORTED_PLUGIN_MAIN = "main.cjs"; +export const LEGACY_IMPORTED_PLUGIN_MAIN = "main.js"; +export const IMPORTED_PLUGIN_WRAPPER_SOURCE = + "// Generated by PI-Desktop: declarative skills and/or agent extensions.\nmodule.exports = {};\n"; + +function isGeneratedImportedWrapper(source: string): boolean { + const normalized = source.replace(/\r\n/g, "\n"); + return /^\/\/ Generated by PI-Desktop:.*\nmodule\.exports = \{\};\n$/.test(normalized); +} + +/** + * Rewrite a pre-`main.cjs` imported plugin wrapper in place (spec 16 §3.2). + * Only the generated no-op is touched; copied package files stay as they are. + */ +export function repairImportedExtensionWrapper(pluginPath: string): boolean { + const manifestPath = join(pluginPath, "manifest.json"); + if (!existsSync(manifestPath)) return false; + let manifest: Record; + try { + manifest = JSON.parse(readFileSync(manifestPath, "utf8")) as Record; + } catch { + return false; + } + if (!manifest || typeof manifest !== "object" || Array.isArray(manifest)) return false; + if (typeof manifest.id !== "string" || !manifest.id.startsWith(IMPORTED_PLUGIN_ID_PREFIX)) { + return false; + } + if (manifest.main !== LEGACY_IMPORTED_PLUGIN_MAIN) return false; + + const legacyPath = join(pluginPath, LEGACY_IMPORTED_PLUGIN_MAIN); + const nextPath = join(pluginPath, IMPORTED_PLUGIN_MAIN); + if (!existsSync(legacyPath)) return false; + if (!isGeneratedImportedWrapper(readFileSync(legacyPath, "utf8"))) return false; + if (existsSync(nextPath) && !isGeneratedImportedWrapper(readFileSync(nextPath, "utf8"))) { + return false; + } + + writeFileSync(nextPath, IMPORTED_PLUGIN_WRAPPER_SOURCE, "utf8"); + writeFileSync( + manifestPath, + `${JSON.stringify({ ...manifest, main: IMPORTED_PLUGIN_MAIN }, null, 2)}\n`, + "utf8", + ); + unlinkSync(legacyPath); + return true; +} diff --git a/apps/desktop/electron/main/plugin-runtime.ts b/apps/desktop/electron/main/plugin-runtime.ts index b2f542174..df087cbc5 100644 --- a/apps/desktop/electron/main/plugin-runtime.ts +++ b/apps/desktop/electron/main/plugin-runtime.ts @@ -94,6 +94,7 @@ import { type PluginShortcutEntry, type PluginShortcutRegistry, } from "./plugin-shortcut-registry"; +import { repairImportedExtensionWrapper } from "./imported-plugin-wrapper"; export type RegisteredCommand = { id: string; @@ -1582,6 +1583,8 @@ export class PluginRuntime { if (!existsSync(manifestPath)) { throw new Error("PLUGIN_INVALID: manifest.json missing"); } + // Generated no-op `main.js` wrappers fail under package `"type":"module"`. + repairImportedExtensionWrapper(pluginPath); const raw = JSON.parse(readFileSync(manifestPath, "utf8")); const validated = validateManifest(raw); if (!validated.ok || !validated.manifest) { diff --git a/apps/desktop/test/agent-extensions.test.mjs b/apps/desktop/test/agent-extensions.test.mjs index aa152387c..ee06b51e2 100644 --- a/apps/desktop/test/agent-extensions.test.mjs +++ b/apps/desktop/test/agent-extensions.test.mjs @@ -12,6 +12,10 @@ const { generateImportedExtensionPlugin, installExtensionDependencies, } = await import("../electron/main/agent-extensions.ts"); +const { + IMPORTED_PLUGIN_WRAPPER_SOURCE, + repairImportedExtensionWrapper, +} = await import("../electron/main/imported-plugin-wrapper.ts"); const { withRegistryOnlyProxy } = await import("../electron/main/npm-registry-proxy.ts"); function bridge(overrides = {}) { @@ -287,6 +291,49 @@ test("importing a pi extension directory or file generates a plugin holding agen assert.throws(() => generateImportedExtensionPlugin(join(root, "notes.md"), importRoot), /no extension entry/); }); +test("repairImportedExtensionWrapper migrates the generated main.js no-op in place", () => { + const root = mkdtempSync(join(tmpdir(), "pi-ax-repair-")); + const dir = join(root, "imported-plugin"); + mkdirSync(dir); + writeFileSync(join(dir, "manifest.json"), JSON.stringify({ + schemaVersion: 1, + id: "imported.git-helper", + name: "git-helper", + version: "0.0.0", + main: "main.js", + }, null, 2) + "\n"); + writeFileSync(join(dir, "main.js"), IMPORTED_PLUGIN_WRAPPER_SOURCE); + writeFileSync(join(dir, "package.json"), JSON.stringify({ name: "x", type: "module" })); + const originalPkg = readFileSync(join(dir, "package.json"), "utf8"); + + assert.equal(repairImportedExtensionWrapper(dir), true); + assert.equal(JSON.parse(readFileSync(join(dir, "manifest.json"), "utf8")).main, "main.cjs"); + assert.equal(readFileSync(join(dir, "main.cjs"), "utf8"), IMPORTED_PLUGIN_WRAPPER_SOURCE); + assert.equal(existsSync(join(dir, "main.js")), false); + assert.equal(readFileSync(join(dir, "package.json"), "utf8"), originalPkg); + assert.equal(repairImportedExtensionWrapper(dir), false); + + writeFileSync(join(dir, "main.js"), "// Generated by PI-Desktop: this plugin only contributes agent extensions.\nmodule.exports = {};\n"); + const legacyComment = JSON.parse(readFileSync(join(dir, "manifest.json"), "utf8")); + legacyComment.main = "main.js"; + writeFileSync(join(dir, "manifest.json"), JSON.stringify(legacyComment, null, 2) + "\n"); + assert.equal(repairImportedExtensionWrapper(dir), true); + assert.equal(JSON.parse(readFileSync(join(dir, "manifest.json"), "utf8")).main, "main.cjs"); + + writeFileSync(join(dir, "main.js"), "module.exports = { custom: true };\n"); + const custom = JSON.parse(readFileSync(join(dir, "manifest.json"), "utf8")); + custom.main = "main.js"; + writeFileSync(join(dir, "manifest.json"), JSON.stringify(custom, null, 2) + "\n"); + assert.equal(repairImportedExtensionWrapper(dir), false); + assert.equal(readFileSync(join(dir, "main.js"), "utf8"), "module.exports = { custom: true };\n"); + + custom.id = "demo.not-imported"; + writeFileSync(join(dir, "manifest.json"), JSON.stringify(custom, null, 2) + "\n"); + writeFileSync(join(dir, "main.js"), IMPORTED_PLUGIN_WRAPPER_SOURCE); + assert.equal(repairImportedExtensionWrapper(dir), false); + rmSync(root, { recursive: true, force: true }); +}); + test("importing a directory keeps its package.json at the plugin root and never copies node_modules", () => { const root = mkdtempSync(join(tmpdir(), "pi-ax-pkg-")); const extDir = join(root, "memory-ext"); diff --git a/apps/desktop/test/imported-package-skills-runtime.test.mjs b/apps/desktop/test/imported-package-skills-runtime.test.mjs index 10b2fa111..a8512ab20 100644 --- a/apps/desktop/test/imported-package-skills-runtime.test.mjs +++ b/apps/desktop/test/imported-package-skills-runtime.test.mjs @@ -195,3 +195,36 @@ test("re-importing an older ESM package creates a loadable copy without rewritin assertSkillCatalog(runtime, imported, bodies, paths); assert.equal(runtime.getAgentExtensions().length, 1); }); + +test("loading an older generated ESM wrapper rewrites it in place and initializes", async (t) => { + const { source, importRoot, runtime, bodies, paths } = createHarness(t, { packageType: "module" }); + const imported = generateImportedExtensionPlugin(source, importRoot); + const oldManifest = JSON.parse(readFileSync(join(imported.path, "manifest.json"), "utf8")); + oldManifest.main = "main.js"; + writeFileSync(join(imported.path, "manifest.json"), JSON.stringify(oldManifest, null, 2) + "\n"); + renameSync(join(imported.path, "main.cjs"), join(imported.path, "main.js")); + const originalPkg = readFileSync(join(imported.path, "package.json"), "utf8"); + + await runtime.loadFromPath(imported.path); + const manifest = runtime.getLoaded(imported.id).manifest; + assert.equal(manifest.main, "main.cjs"); + assert.ok(existsSync(join(imported.path, "main.cjs"))); + assert.equal(existsSync(join(imported.path, "main.js")), false); + assert.equal(readFileSync(join(imported.path, "package.json"), "utf8"), originalPkg); + assertSkillCatalog(runtime, imported, bodies, paths); + assert.equal(runtime.getAgentExtensions().length, 1); +}); + +test("loading a customized imported main.js does not rewrite the wrapper", async (t) => { + const { source, importRoot, runtime } = createHarness(t, { packageType: "module" }); + const imported = generateImportedExtensionPlugin(source, importRoot); + const oldManifest = JSON.parse(readFileSync(join(imported.path, "manifest.json"), "utf8")); + oldManifest.main = "main.js"; + writeFileSync(join(imported.path, "manifest.json"), JSON.stringify(oldManifest, null, 2) + "\n"); + writeFileSync(join(imported.path, "main.js"), "module.exports = { custom: true };\n"); + const originalJs = readFileSync(join(imported.path, "main.js"), "utf8"); + + await assert.rejects(runtime.loadFromPath(imported.path), /module is not defined in ES module scope/); + assert.equal(JSON.parse(readFileSync(join(imported.path, "manifest.json"), "utf8")).main, "main.js"); + assert.equal(readFileSync(join(imported.path, "main.js"), "utf8"), originalJs); +}); diff --git a/docs/spec/06-delivery/04-e2e-test-plan.md b/docs/spec/06-delivery/04-e2e-test-plan.md index abb6a0b18..5ba942567 100644 --- a/docs/spec/06-delivery/04-e2e-test-plan.md +++ b/docs/spec/06-delivery/04-e2e-test-plan.md @@ -11822,12 +11822,15 @@ plugin-form fixtures in an isolated temporary directory at runtime. - **Steps**: Run `node --test apps/desktop/test/imported-package-skills-runtime.test.mjs`. Generate each plugin through the production importer, load it through `PluginRuntime` and the real child-process plugin host, read its skill catalog - and bodies, and inspect its declared extension. Re-import the older fixture. + and bodies, and inspect its declared extension. Re-import the older fixture + without loading it, then load the older fixture itself. - **Expected**: Each new manifest points to an existing `main.cjs`; initialization succeeds for all three package types. Source and both copied `package.json` files retain the same bytes. The repeated import has a distinct path/id and - loads successfully; the older manifest, wrapper, and package files remain - unchanged. There is no automatic migration of older imported plugins. + loads successfully; generating it leaves the older manifest, wrapper, and + package files unchanged. Loading the older fixture rewrites its generated + `main.js` in place to `main.cjs`, keeps the copied `package.json` bytes, and + initializes. A customized `main.js` is not rewritten. - **Specs linked**: `07-plugins/16-trusted-extensions.md` §3.2; ADR 0215. - **Acceptance**: Quality - **Status**: Automated import-to-plugin-host fixture. Run against the committed diff --git a/docs/spec/07-plugins/16-trusted-extensions.md b/docs/spec/07-plugins/16-trusted-extensions.md index 712ad4e83..65bdcfa2c 100644 --- a/docs/spec/07-plugins/16-trusted-extensions.md +++ b/docs/spec/07-plugins/16-trusted-extensions.md @@ -84,11 +84,13 @@ local-plugin flow. The confirmation before the picker remains the trust decision; the generated manifest declares the permissions needed by its actual contributions. The manifest's `main` points to `main.cjs` regardless of the source package's `type`; both copied package declarations retain their module -semantics. Existing imported directories are not rewritten on upgrade. To -repair an older import whose generated `main.js` fails under `type: module`, -remove that failed imported plugin and import its source again. Re-importing -without removal creates a separate plugin with a unique suffix and leaves the -old copy unchanged; it does not migrate its grants or activation scope. +semantics. Loading an imported plugin whose `main` is the generated CommonJS +`main.js` wrapper rewrites that file in place to `main.cjs` and updates the +manifest; copied package files, grants, and activation scope stay as they are. +The rewrite matches only the generated no-op (including the original comment +text). A customized `main.js` is left untouched. Re-importing without removal +creates a separate plugin with a unique suffix; it does not copy grants or +activation scope from the older copy. For extension files and packages without `pi.skills`, entry discovery keeps the existing `pi-coding-agent` rules: `package.json` `pi.extensions`, otherwise diff --git a/docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md b/docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md index 892bbe233..9374ead94 100644 --- a/docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md +++ b/docs/zh-CN/spec/06-delivery/04-e2e-test-plan.md @@ -6988,10 +6988,12 @@ runner 会在运行时的隔离临时目录中生成六个插件形态 fixture CommonJS `main.js` 及指向它的 manifest。 - **步骤**：运行 `node --test apps/desktop/test/imported-package-skills-runtime.test.mjs`。 使用生产导入器生成各个插件，通过 `PluginRuntime` 与真实子进程插件宿主加载， - 读取技能目录与正文，并检查声明的扩展。对旧版夹具重新导入。 + 读取技能目录与正文，并检查声明的扩展。对旧版夹具重新导入且不加载旧副本，再加载旧副本。 - **预期**：每份新 manifest 都指向实际存在的 `main.cjs`，三类包均初始化成功。 源包及两份复制的 `package.json` 字节保持一致。重新导入得到不同的路径和 id 并成功 - 加载；旧 manifest、包装器和包文件保持原样。不会自动迁移既有导入插件。 + 加载；生成新副本时旧 manifest、包装器和包文件保持原样。加载旧副本会把生成的 + `main.js` 就地改写为 `main.cjs`，复制的 `package.json` 字节不变，并初始化成功。 + 自定义过的 `main.js` 不会被改写。 - **关联规范**：`07-plugins/16-trusted-extensions.md` §3.2；ADR 0215。 - **验收**：质量 - **状态**：已实现导入到插件宿主的自动化夹具。合入最新 `origin/main` 后，在已提交的 diff --git a/docs/zh-CN/spec/07-plugins/16-trusted-extensions.md b/docs/zh-CN/spec/07-plugins/16-trusted-extensions.md index e5747ce1f..387262a64 100644 --- a/docs/zh-CN/spec/07-plugins/16-trusted-extensions.md +++ b/docs/zh-CN/spec/07-plugins/16-trusted-extensions.md @@ -66,9 +66,11 @@ agent 循环上注册工具、命令和事件处理器。`ExtensionAPI` 契约 CommonJS `main.cjs` 和 id 为 `imported.` 的 manifest（重复导入时追加唯一后缀），再通过 既有本地插件流程注册。选择器之前的确认仍是信任决定；生成的 manifest 只声明实际贡献 所需的权限。无论源包的 `type` 为何，manifest 的 `main` 都指向 `main.cjs`； -两份复制的包声明保留原有模块语义。升级不会重写既有导入目录。若旧导入的生成文件 -`main.js` 因 `type: module` 而加载失败，删除该失败插件后重新导入源包即可。 -不删除就重新导入会创建带唯一后缀的独立插件，旧副本保持原样；不会迁移其授权或激活范围。 +两份复制的包声明保留原有模块语义。加载时若导入插件的 `main` 仍是生成的 CommonJS +`main.js` 空包装器，则就地改写为 `main.cjs` 并更新 manifest；复制的包文件、授权和 +激活范围保持不变。只匹配生成的空操作（含最初的注释文本）。自定义过的 `main.js` +不会改动。不删除就重新导入仍会创建带唯一后缀的独立插件，不会从旧副本复制授权或 +激活范围。 扩展文件及未声明 `pi.skills` 的包保持既有 `pi-coding-agent` 入口发现规则：先取 `package.json` 的 `pi.extensions`，否则取 `index.ts` / `index.js`，再否则取一层深度内