From 33340f82ce39b209800e5a4fc9d967b7d8c45406 Mon Sep 17 00:00:00 2001 From: roboomp Date: Thu, 16 Jul 2026 05:41:35 +0000 Subject: [PATCH] fix(plugins): restored legacy commonjs compatibility - Added DefaultPackageManager discovery compatibility for legacy extensions. - Loaded graph-owned CommonJS modules through synchronous default bridges. Fixes #5658 --- packages/coding-agent/CHANGELOG.md | 4 + .../legacy-pi-coding-agent-shim.ts | 97 ++++++++++++++++++- .../extensibility/plugins/legacy-pi-compat.ts | 96 ++++++++++++++---- .../legacy-pi-default-resource-loader.test.ts | 30 ++++++ .../legacy-pi-inplace-load.test.ts | 30 ++++++ 5 files changed, 236 insertions(+), 21 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 0e753ced7..5ae1bcee0 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed linked legacy pi extensions failing to load when they import `DefaultPackageManager` or linkedom: the coding-agent compatibility shim now enumerates OMP extension paths with plugin metadata, and extension-graph CommonJS modules load through synchronous default-export bridges with linkedom's bundled canvas fallback. ([#5658](https://github.com/can1357/oh-my-pi/issues/5658)) + ## [17.0.1] - 2026-07-16 ### Changed diff --git a/packages/coding-agent/src/extensibility/legacy-pi-coding-agent-shim.ts b/packages/coding-agent/src/extensibility/legacy-pi-coding-agent-shim.ts index ec9e78916..8e72f2d49 100644 --- a/packages/coding-agent/src/extensibility/legacy-pi-coding-agent-shim.ts +++ b/packages/coding-agent/src/extensibility/legacy-pi-coding-agent-shim.ts @@ -44,9 +44,10 @@ import { ReadTool } from "../tools/read"; import { formatBytes } from "../tools/render-utils"; import { WriteTool } from "../tools/write"; import { EventBus } from "../utils/event-bus"; -import { loadExtensionFromFactory, loadExtensions } from "./extensions"; +import { discoverExtensionPaths, loadExtensionFromFactory, loadExtensions } from "./extensions"; import { ExtensionRuntime } from "./extensions/loader"; import type { ExtensionFactory, ToolDefinition } from "./extensions/types"; +import { getEnabledPlugins, resolvePluginExtensionPaths, type ScopedInstalledPlugin } from "./plugins/loader"; import type { Skill } from "./skills"; import { loadSkillsFromDir } from "./skills"; import { Type } from "./typebox"; @@ -655,6 +656,100 @@ export const SettingsManager = { }, } as const; +/** Scope used by the legacy package manager for discovered resources. */ +export type SourceScope = "user" | "project" | "temporary"; + +/** Discovery metadata exposed alongside a legacy package resource path. */ +export interface PathMetadata { + source: string; + scope: SourceScope; + origin: "package" | "top-level"; + baseDir?: string; +} + +/** One extension, skill, prompt, or theme resolved by the legacy package manager. */ +export interface ResolvedResource { + path: string; + enabled: boolean; + metadata: PathMetadata; +} + +/** Resource groups returned by {@link DefaultPackageManager.resolve}. */ +export interface ResolvedPaths { + extensions: ResolvedResource[]; + skills: ResolvedResource[]; + prompts: ResolvedResource[]; + themes: ResolvedResource[]; +} + +/** Action a legacy caller requests when a configured package is unavailable. */ +export type MissingSourceAction = "install" | "skip" | "error"; + +/** Construction inputs accepted by the legacy package manager. */ +export interface DefaultPackageManagerOptions { + cwd: string; + agentDir: string; + settingsManager: Settings | Promise; +} + +/** + * Enumerates the extensions OMP would load through the historical package + * manager surface used by legacy extensions. + */ +export class DefaultPackageManager { + #cwd: string; + #agentDir: string; + #settingsManager: Settings | Promise; + + constructor(options: DefaultPackageManagerOptions) { + this.#cwd = options.cwd; + this.#agentDir = options.agentDir; + this.#settingsManager = options.settingsManager; + } + + /** Resolve enabled extension paths with their OMP plugin provenance. */ + async resolve(_onMissing?: (source: string) => Promise): Promise { + const settings = await this.#settingsManager; + const configuredPaths = settings.get("extensions") ?? []; + const disabledExtensionIds = settings.get("disabledExtensions") ?? []; + const [extensionPaths, plugins] = await Promise.all([ + discoverExtensionPaths(configuredPaths, this.#cwd, disabledExtensionIds), + getEnabledPlugins(this.#cwd), + ]); + const pluginByExtensionPath = new Map(); + for (const plugin of plugins) { + for (const extensionPath of resolvePluginExtensionPaths(plugin)) { + pluginByExtensionPath.set(path.resolve(extensionPath), plugin); + } + } + + const extensions = extensionPaths.map(extensionPath => { + const resolvedPath = path.resolve(extensionPath); + const plugin = pluginByExtensionPath.get(resolvedPath); + const agentDirRelative = path.relative(path.resolve(this.#agentDir), resolvedPath); + const metadata: PathMetadata = plugin + ? { + source: `npm:${plugin.name}`, + scope: plugin.scope, + origin: "package", + baseDir: plugin.path, + } + : { + source: "auto", + scope: + agentDirRelative === "" || + (!agentDirRelative.startsWith("..") && !path.isAbsolute(agentDirRelative)) + ? "user" + : "project", + origin: "top-level", + }; + return { path: resolvedPath, enabled: true, metadata }; + }); + + return { extensions, skills: [], prompts: [], themes: [] }; + } +} + /** * Resource-loader compatibility layer for legacy pi extensions. * diff --git a/packages/coding-agent/src/extensibility/plugins/legacy-pi-compat.ts b/packages/coding-agent/src/extensibility/plugins/legacy-pi-compat.ts index 361266110..652e04f78 100644 --- a/packages/coding-agent/src/extensibility/plugins/legacy-pi-compat.ts +++ b/packages/coding-agent/src/extensibility/plugins/legacy-pi-compat.ts @@ -1,6 +1,6 @@ /// import * as fs from "node:fs"; -import { isBuiltin } from "node:module"; +import { createRequire, isBuiltin } from "node:module"; import * as path from "node:path"; import * as url from "node:url"; import { isCompiledBinary, stripWindowsExtendedLengthPathPrefix } from "@oh-my-pi/pi-utils"; @@ -1111,6 +1111,9 @@ const EXTENSION_GRAPH_SPECIFIER_REGEX = /((?:from\s+|import\s+|import\s*\(\s*)[" // reloads install supplemental hooks only for modules added to the graph since // the previous load. const extensionGraphHookModules = new Map>(); +const COMMONJS_MODULES_GLOBAL = "__ompLegacyPiCommonJsModules"; +const commonJsModuleExports = new Map(); +Reflect.set(globalThis, COMMONJS_MODULES_GLOBAL, commonJsModuleExports); let legacyPiLoadTag = 0; @@ -1144,9 +1147,9 @@ async function realpathOrSelfUncached(p: string): Promise { * Extension-local bare dependency entries are also included so their relative * children receive the reload mtime tag; bare imports inside those dependencies * remain native Bun resolutions to avoid taking over full third-party graphs. - * CommonJS modules reached through `require()` stay on Bun's native loader. - * The only exception is a module whose bare requires resolve to native addons: - * those require a synchronous hook that pins the addon to an absolute path. + * CommonJS modules reached through `require()` stay on Bun's native loader + * unless they resolve native addons. CommonJS reached through ESM imports stays + * graph-owned so the load hook can expose its exports through an ESM default. */ async function collectExtensionModules(entryRealPath: string): Promise> { const modules = new Map(); @@ -1254,20 +1257,51 @@ async function collectExtensionModules(entryRealPath: string): Promise { + const packageRoot = await findPackageRoot(modulePath); + const packageJsonPath = packageRoot ? path.join(packageRoot, "package.json") : modulePath; + let targetPath = modulePath; + if (packageRoot) { + const manifest = await readPackageManifest(packageRoot); + const packageRelativePath = path.relative(packageRoot, modulePath).split(path.sep).join("/"); + if (manifest?.name === "linkedom" && packageRelativePath === "commonjs/canvas.cjs") { + targetPath = path.join(packageRoot, "commonjs", "canvas-shim.cjs"); + } + } + + const requireFromPackage = createRequire(packageJsonPath); + const specifier = packageRoot ? `./${path.relative(packageRoot, targetPath).split(path.sep).join("/")}` : targetPath; + commonJsModuleExports.set(modulePath, requireFromPackage(specifier)); + return `export default globalThis[${JSON.stringify(COMMONJS_MODULES_GLOBAL)}].get(${JSON.stringify(modulePath)});\n`; +} + +/** + * Install exact-path load hooks for the current extension graph. ESM/TS source + * retains the async rewrite path. CommonJS wrappers and native-addon loaders + * stay synchronous because Bun rejects `require()` targets backed by async + * `onLoad` callbacks. + */ +async function installExtensionGraphHook( entryRealPath: string, modules: Map, -): { asyncModules: Map; syncCommonJsModules: Map } { +): Promise<{ asyncModules: Map; syncSourceModules: Map }> { const asyncModules = new Map(); - const syncCommonJsModules = new Map(); + const commonJsModules = new Map(); + const syncSourceModules = new Map(); for (const [modulePath, source] of modules) { - const destination = nativeAddonLoaderModulePaths.has(modulePath) ? syncCommonJsModules : asyncModules; - destination.set(modulePath, source); + const extension = path.extname(modulePath); + if (extension === ".cjs" || extension === ".cts") { + commonJsModules.set(modulePath, await synthesizeCommonJsDefaultModule(modulePath)); + } else if (nativeAddonLoaderModulePaths.has(modulePath)) { + syncSourceModules.set(modulePath, source); + } else { + asyncModules.set(modulePath, source); + } } if (asyncModules.size > 0) { @@ -1299,17 +1333,39 @@ function installExtensionGraphHook( }); } - if (syncCommonJsModules.size > 0) { - const alternation = [...syncCommonJsModules.keys()].map(escapeRegExp).join("|"); + if (commonJsModules.size > 0) { + const alternation = [...commonJsModules.keys()].map(escapeRegExp).join("|"); const filter = new RegExp(`^(?:${alternation})(?:\\?mtime=\\d+)?$`); - const hookId = Bun.hash(`${entryRealPath}\0sync-cjs\0${[...syncCommonJsModules.keys()].join("\0")}`).toString(36); + const hookId = Bun.hash(`${entryRealPath}\0commonjs\0${[...commonJsModules.keys()].join("\0")}`).toString(36); Bun.plugin({ name: `omp:legacy-pi-ext:${hookId}`, setup(build) { build.onLoad({ filter, namespace: "file" }, args => { const queryIndex = args.path.indexOf("?mtime="); const sourcePath = queryIndex >= 0 ? args.path.slice(0, queryIndex) : args.path; - const source = syncCommonJsModules.get(sourcePath); + const source = commonJsModules.get(sourcePath); + if (source === undefined) { + throw new Error(`Missing CommonJS compatibility module: ${sourcePath}`); + } + return { contents: source, loader: "js" }; + }); + }, + }); + } + + if (syncSourceModules.size > 0) { + const alternation = [...syncSourceModules.keys()].map(escapeRegExp).join("|"); + const filter = new RegExp(`^(?:${alternation})(?:\\?mtime=\\d+)?$`); + const hookId = Bun.hash(`${entryRealPath}\0sync-source\0${[...syncSourceModules.keys()].join("\0")}`).toString( + 36, + ); + Bun.plugin({ + name: `omp:legacy-pi-ext:${hookId}`, + setup(build) { + build.onLoad({ filter, namespace: "file" }, args => { + const queryIndex = args.path.indexOf("?mtime="); + const sourcePath = queryIndex >= 0 ? args.path.slice(0, queryIndex) : args.path; + const source = syncSourceModules.get(sourcePath); if (source === undefined) { throw new Error(`Missing pre-rewritten CommonJS extension source: ${sourcePath}`); } @@ -1318,7 +1374,7 @@ function installExtensionGraphHook( }, }); } - return { asyncModules, syncCommonJsModules }; + return { asyncModules, syncSourceModules }; } /** @@ -1347,14 +1403,14 @@ async function ensureExtensionGraphHook(entryRealPath: string): Promise<{ clear( return undefined; } - const { asyncModules, syncCommonJsModules } = installExtensionGraphHook(entryRealPath, pendingModules); + const { asyncModules, syncSourceModules } = await installExtensionGraphHook(entryRealPath, pendingModules); for (const modulePath of pendingModules.keys()) { hookedModules.add(modulePath); } return { clear() { asyncModules.clear(); - syncCommonJsModules.clear(); + syncSourceModules.clear(); }, }; } diff --git a/packages/coding-agent/test/extensibility/legacy-pi-default-resource-loader.test.ts b/packages/coding-agent/test/extensibility/legacy-pi-default-resource-loader.test.ts index f180a3af9..c489e0038 100644 --- a/packages/coding-agent/test/extensibility/legacy-pi-default-resource-loader.test.ts +++ b/packages/coding-agent/test/extensibility/legacy-pi-default-resource-loader.test.ts @@ -4,6 +4,7 @@ import * as os from "node:os"; import * as path from "node:path"; import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { + DefaultPackageManager, DefaultResourceLoader, createAgentSession as legacyCreateAgentSession, } from "@oh-my-pi/pi-coding-agent/extensibility/legacy-pi-coding-agent-shim"; @@ -41,6 +42,35 @@ async function mkTempCwd(prefix: string): Promise { return dir; } +describe("DefaultPackageManager.resolve() (issue #5658)", () => { + it("enumerates configured extension paths through OMP discovery", async () => { + const tmp = await mkTempCwd("omp-legacy-default-package-manager-"); + const cwd = path.join(tmp, "project"); + const agentDir = path.join(tmp, "agent"); + const extensionPath = path.join(cwd, "configured-extension.ts"); + await fs.mkdir(cwd, { recursive: true }); + await fs.writeFile(extensionPath, "export default function () {}\n", "utf8"); + const settingsManager = Settings.isolated({ extensions: [extensionPath] }); + const manager = new DefaultPackageManager({ cwd, agentDir, settingsManager }); + + const resolved = await manager.resolve(() => Promise.resolve("skip")); + const extension = resolved.extensions.find(resource => resource.path === extensionPath); + + expect(extension).toEqual({ + path: extensionPath, + enabled: true, + metadata: { + source: "auto", + scope: "project", + origin: "top-level", + }, + }); + expect(resolved.skills).toEqual([]); + expect(resolved.prompts).toEqual([]); + expect(resolved.themes).toEqual([]); + }); +}); + describe("DefaultResourceLoader.reload() (issue #4567)", () => { it("populates the discovery snapshot honoring no* flags and applying every override", async () => { const tmp = await mkTempCwd("omp-legacy-default-resource-loader-reload-"); diff --git a/packages/coding-agent/test/extensibility/legacy-pi-inplace-load.test.ts b/packages/coding-agent/test/extensibility/legacy-pi-inplace-load.test.ts index fe1717ab3..116f54a3e 100644 --- a/packages/coding-agent/test/extensibility/legacy-pi-inplace-load.test.ts +++ b/packages/coding-agent/test/extensibility/legacy-pi-inplace-load.test.ts @@ -78,6 +78,36 @@ describe("legacy-pi in-place module loading (issue #1674)", () => { expect(mod.value).toBe("config-ok"); }); + it("loads a default import from linkedom's CommonJS canvas fallback", async () => { + const dir = await writePackage({ + "package.json": JSON.stringify({ name: "linkedom-consumer", version: "1.0.0", type: "module" }), + "index.js": 'export { canvasValue } from "linkedom";\n', + "node_modules/linkedom/package.json": JSON.stringify({ + name: "linkedom", + version: "0.18.12", + type: "module", + exports: "./index.js", + }), + "node_modules/linkedom/index.js": [ + 'import Canvas from "./commonjs/canvas.cjs";', + "export const canvasValue = Canvas.createCanvas();", + ].join("\n"), + "node_modules/linkedom/commonjs/canvas.cjs": [ + "try {", + ' module.exports = require("canvas");', + "} catch {", + ' module.exports = require("./canvas-shim.cjs");', + "}", + ].join("\n"), + "node_modules/linkedom/commonjs/canvas-shim.cjs": + 'module.exports = { createCanvas: () => "linkedom-canvas-shim" };\n', + }); + + const mod = await loadLegacyPiModule(path.join(dir, "index.js")); + + expect(Reflect.get(Object(mod), "canvasValue")).toBe("linkedom-canvas-shim"); + }); + it("reloads an edited entry module without polluting fileURLToPath-derived paths", async () => { const entrySource = (version: string): string => [