From 4a84412b3a1f89a2a7b9c78bb0a8eb8dfec18fbf Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 6 Apr 2026 16:50:46 +0100 Subject: [PATCH] refactor: dedupe channel plugin helpers --- .../plugins/bundled.shape-guard.test.ts | 1 + src/channels/plugins/bundled.ts | 96 +++------------- src/channels/plugins/module-loader.test.ts | 65 +++++++++++ src/channels/plugins/module-loader.ts | 108 ++++++++++++++++++ src/channels/plugins/package-state-probes.ts | 100 ++-------------- 5 files changed, 201 insertions(+), 169 deletions(-) create mode 100644 src/channels/plugins/module-loader.test.ts create mode 100644 src/channels/plugins/module-loader.ts diff --git a/src/channels/plugins/bundled.shape-guard.test.ts b/src/channels/plugins/bundled.shape-guard.test.ts index 8c2da1b2d6b..f43c6590592 100644 --- a/src/channels/plugins/bundled.shape-guard.test.ts +++ b/src/channels/plugins/bundled.shape-guard.test.ts @@ -6,6 +6,7 @@ import { importFreshModule } from "../../../test/helpers/import-fresh.ts"; import { loadPluginManifestRegistry } from "../../plugins/manifest-registry.js"; afterEach(() => { + vi.resetModules(); vi.doUnmock("../../plugins/discovery.js"); vi.doUnmock("../../plugins/manifest-registry.js"); vi.doUnmock("../../infra/boundary-file-read.js"); diff --git a/src/channels/plugins/bundled.ts b/src/channels/plugins/bundled.ts index 0f7f221b088..8716e3118c4 100644 --- a/src/channels/plugins/bundled.ts +++ b/src/channels/plugins/bundled.ts @@ -1,8 +1,4 @@ -import fs from "node:fs"; -import { createRequire } from "node:module"; import path from "node:path"; -import { createJiti } from "jiti"; -import { openBoundaryFileSync } from "../../infra/boundary-file-read.js"; import { createSubsystemLogger } from "../../logging/subsystem.js"; import type { BundledChannelEntryContract, @@ -11,10 +7,10 @@ import type { import { loadPluginManifestRegistry } from "../../plugins/manifest-registry.js"; import type { PluginRuntime } from "../../plugins/runtime/types.js"; import { - buildPluginLoaderAliasMap, - buildPluginLoaderJitiOptions, - shouldPreferNativeJiti, -} from "../../plugins/sdk-alias.js"; + isJavaScriptModulePath, + loadChannelPluginModule, + resolveCompiledBundledModulePath, +} from "./module-loader.js"; import type { ChannelId, ChannelPlugin } from "./types.js"; type GeneratedBundledChannelEntry = { @@ -24,7 +20,6 @@ type GeneratedBundledChannelEntry = { }; const log = createSubsystemLogger("channels"); -const nodeRequire = createRequire(import.meta.url); function resolveChannelPluginModuleEntry( moduleExport: unknown, @@ -76,70 +71,6 @@ function resolveChannelSetupModuleEntry( return record as BundledChannelSetupEntryContract; } -function createModuleLoader() { - const jitiLoaders = new Map>(); - - return (modulePath: string) => { - const tryNative = - shouldPreferNativeJiti(modulePath) || modulePath.includes(`${path.sep}dist${path.sep}`); - const aliasMap = buildPluginLoaderAliasMap(modulePath, process.argv[1], import.meta.url); - const cacheKey = JSON.stringify({ - tryNative, - aliasMap: Object.entries(aliasMap).toSorted(([left], [right]) => left.localeCompare(right)), - }); - const cached = jitiLoaders.get(cacheKey); - if (cached) { - return cached; - } - const loader = createJiti(import.meta.url, { - ...buildPluginLoaderJitiOptions(aliasMap), - tryNative, - }); - jitiLoaders.set(cacheKey, loader); - return loader; - }; -} - -const loadModule = createModuleLoader(); - -function loadBundledModule(modulePath: string, rootDir: string): unknown { - const boundaryRoot = resolveCompiledBundledModulePath(rootDir); - const opened = openBoundaryFileSync({ - absolutePath: modulePath, - rootPath: boundaryRoot, - boundaryLabel: "plugin root", - rejectHardlinks: false, - skipLexicalRootCheck: true, - }); - if (!opened.ok) { - throw new Error("plugin entry path escapes plugin root or fails alias checks"); - } - const safePath = opened.path; - fs.closeSync(opened.fd); - if ( - process.platform === "win32" && - safePath.includes(`${path.sep}dist${path.sep}`) && - [".js", ".mjs", ".cjs"].includes(path.extname(safePath).toLowerCase()) - ) { - try { - return nodeRequire(safePath); - } catch { - // Fall back to the Jiti loader path when require() cannot handle the entry. - } - } - return loadModule(safePath)(safePath); -} - -function resolveCompiledBundledModulePath(modulePath: string): string { - const compiledDistModulePath = modulePath.replace( - `${path.sep}dist-runtime${path.sep}`, - `${path.sep}dist${path.sep}`, - ); - return compiledDistModulePath !== modulePath && fs.existsSync(compiledDistModulePath) - ? compiledDistModulePath - : modulePath; -} - function loadGeneratedBundledChannelEntries(): readonly GeneratedBundledChannelEntry[] { const manifestRegistry = loadPluginManifestRegistry({ cache: false, config: {} }); const entries: GeneratedBundledChannelEntry[] = []; @@ -152,7 +83,13 @@ function loadGeneratedBundledChannelEntries(): readonly GeneratedBundledChannelE try { const sourcePath = resolveCompiledBundledModulePath(manifest.source); const entry = resolveChannelPluginModuleEntry( - loadBundledModule(sourcePath, manifest.rootDir), + loadChannelPluginModule({ + modulePath: sourcePath, + rootDir: manifest.rootDir, + boundaryRootDir: resolveCompiledBundledModulePath(manifest.rootDir), + shouldTryNativeRequire: (safePath) => + safePath.includes(`${path.sep}dist${path.sep}`) && isJavaScriptModulePath(safePath), + }), ); if (!entry) { log.warn( @@ -162,10 +99,13 @@ function loadGeneratedBundledChannelEntries(): readonly GeneratedBundledChannelE } const setupEntry = manifest.setupSource ? resolveChannelSetupModuleEntry( - loadBundledModule( - resolveCompiledBundledModulePath(manifest.setupSource), - manifest.rootDir, - ), + loadChannelPluginModule({ + modulePath: resolveCompiledBundledModulePath(manifest.setupSource), + rootDir: manifest.rootDir, + boundaryRootDir: resolveCompiledBundledModulePath(manifest.rootDir), + shouldTryNativeRequire: (safePath) => + safePath.includes(`${path.sep}dist${path.sep}`) && isJavaScriptModulePath(safePath), + }), ) : null; entries.push({ diff --git a/src/channels/plugins/module-loader.test.ts b/src/channels/plugins/module-loader.test.ts new file mode 100644 index 00000000000..84fa0c0425a --- /dev/null +++ b/src/channels/plugins/module-loader.test.ts @@ -0,0 +1,65 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { + isJavaScriptModulePath, + resolveCompiledBundledModulePath, + resolveExistingPluginModulePath, + resolvePluginModuleCandidates, +} from "./module-loader.js"; + +const tempDirs: string[] = []; + +afterEach(() => { + for (const tempDir of tempDirs.splice(0)) { + fs.rmSync(tempDir, { recursive: true, force: true }); + } +}); + +function createTempDir(): string { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-channel-module-loader-")); + tempDirs.push(tempDir); + return tempDir; +} + +describe("channel plugin module loader helpers", () => { + it("prefers compiled bundled dist output when present", () => { + const rootDir = createTempDir(); + const runtimePath = path.join(rootDir, "dist-runtime", "entry.js"); + const compiledPath = path.join(rootDir, "dist", "entry.js"); + fs.mkdirSync(path.dirname(compiledPath), { recursive: true }); + fs.writeFileSync(compiledPath, "export {};\n", "utf8"); + + expect(resolveCompiledBundledModulePath(runtimePath)).toBe(compiledPath); + }); + + it("keeps dist-runtime path when compiled bundled output is absent", () => { + const rootDir = createTempDir(); + const runtimePath = path.join(rootDir, "dist-runtime", "entry.js"); + + expect(resolveCompiledBundledModulePath(runtimePath)).toBe(runtimePath); + }); + + it("resolves plugin module candidates and picks the first existing extension", () => { + const rootDir = createTempDir(); + const expectedPath = path.join(rootDir, "src", "checker.mjs"); + fs.mkdirSync(path.dirname(expectedPath), { recursive: true }); + fs.writeFileSync(expectedPath, "export const ok = true;\n", "utf8"); + + expect(resolvePluginModuleCandidates(rootDir, "./src/checker")).toEqual([ + path.join(rootDir, "src", "checker"), + path.join(rootDir, "src", "checker.ts"), + path.join(rootDir, "src", "checker.js"), + path.join(rootDir, "src", "checker.mjs"), + path.join(rootDir, "src", "checker.cjs"), + ]); + expect(resolveExistingPluginModulePath(rootDir, "./src/checker")).toBe(expectedPath); + }); + + it("detects JavaScript module paths case-insensitively", () => { + expect(isJavaScriptModulePath("/tmp/entry.js")).toBe(true); + expect(isJavaScriptModulePath("/tmp/entry.MJS")).toBe(true); + expect(isJavaScriptModulePath("/tmp/entry.ts")).toBe(false); + }); +}); diff --git a/src/channels/plugins/module-loader.ts b/src/channels/plugins/module-loader.ts new file mode 100644 index 00000000000..d49cb8189fd --- /dev/null +++ b/src/channels/plugins/module-loader.ts @@ -0,0 +1,108 @@ +import fs from "node:fs"; +import { createRequire } from "node:module"; +import path from "node:path"; +import { createJiti } from "jiti"; +import { openBoundaryFileSync } from "../../infra/boundary-file-read.js"; +import { + buildPluginLoaderAliasMap, + buildPluginLoaderJitiOptions, + shouldPreferNativeJiti, +} from "../../plugins/sdk-alias.js"; + +const nodeRequire = createRequire(import.meta.url); + +function createModuleLoader() { + const jitiLoaders = new Map>(); + + return (modulePath: string) => { + const tryNative = + shouldPreferNativeJiti(modulePath) || modulePath.includes(`${path.sep}dist${path.sep}`); + const aliasMap = buildPluginLoaderAliasMap(modulePath, process.argv[1], import.meta.url); + const cacheKey = JSON.stringify({ + tryNative, + aliasMap: Object.entries(aliasMap).toSorted(([left], [right]) => left.localeCompare(right)), + }); + const cached = jitiLoaders.get(cacheKey); + if (cached) { + return cached; + } + const loader = createJiti(import.meta.url, { + ...buildPluginLoaderJitiOptions(aliasMap), + tryNative, + }); + jitiLoaders.set(cacheKey, loader); + return loader; + }; +} + +let loadModule = createModuleLoader(); + +export function isJavaScriptModulePath(modulePath: string): boolean { + return [".js", ".mjs", ".cjs"].includes(path.extname(modulePath).toLowerCase()); +} + +export function resolveCompiledBundledModulePath(modulePath: string): string { + const compiledDistModulePath = modulePath.replace( + `${path.sep}dist-runtime${path.sep}`, + `${path.sep}dist${path.sep}`, + ); + return compiledDistModulePath !== modulePath && fs.existsSync(compiledDistModulePath) + ? compiledDistModulePath + : modulePath; +} + +export function resolvePluginModuleCandidates(rootDir: string, specifier: string): string[] { + const normalizedSpecifier = specifier.replace(/\\/g, "/"); + const resolvedPath = path.resolve(rootDir, normalizedSpecifier); + const ext = path.extname(resolvedPath); + if (ext) { + return [resolvedPath]; + } + return [ + resolvedPath, + `${resolvedPath}.ts`, + `${resolvedPath}.js`, + `${resolvedPath}.mjs`, + `${resolvedPath}.cjs`, + ]; +} + +export function resolveExistingPluginModulePath(rootDir: string, specifier: string): string { + for (const candidate of resolvePluginModuleCandidates(rootDir, specifier)) { + if (fs.existsSync(candidate)) { + return candidate; + } + } + return path.resolve(rootDir, specifier); +} + +export function loadChannelPluginModule(params: { + modulePath: string; + rootDir: string; + boundaryRootDir?: string; + boundaryLabel?: string; + shouldTryNativeRequire?: (safePath: string) => boolean; +}): unknown { + const opened = openBoundaryFileSync({ + absolutePath: params.modulePath, + rootPath: params.boundaryRootDir ?? params.rootDir, + boundaryLabel: params.boundaryLabel ?? "plugin root", + rejectHardlinks: false, + skipLexicalRootCheck: true, + }); + if (!opened.ok) { + throw new Error( + `${params.boundaryLabel ?? "plugin"} module path escapes plugin root or fails alias checks`, + ); + } + const safePath = opened.path; + fs.closeSync(opened.fd); + if (process.platform === "win32" && params.shouldTryNativeRequire?.(safePath)) { + try { + return nodeRequire(safePath); + } catch { + // Fall back to the Jiti loader path when require() cannot handle the entry. + } + } + return loadModule(safePath)(safePath); +} diff --git a/src/channels/plugins/package-state-probes.ts b/src/channels/plugins/package-state-probes.ts index ac30afbd6bf..6391c5b2cf3 100644 --- a/src/channels/plugins/package-state-probes.ts +++ b/src/channels/plugins/package-state-probes.ts @@ -1,19 +1,14 @@ -import fs from "node:fs"; -import { createRequire } from "node:module"; -import path from "node:path"; -import { createJiti } from "jiti"; import type { OpenClawConfig } from "../../config/config.js"; -import { openBoundaryFileSync } from "../../infra/boundary-file-read.js"; import { createSubsystemLogger } from "../../logging/subsystem.js"; import { listChannelCatalogEntries, type PluginChannelCatalogEntry, } from "../../plugins/channel-catalog-registry.js"; import { - buildPluginLoaderAliasMap, - buildPluginLoaderJitiOptions, - shouldPreferNativeJiti, -} from "../../plugins/sdk-alias.js"; + isJavaScriptModulePath, + loadChannelPluginModule, + resolveExistingPluginModulePath, +} from "./module-loader.js"; type ChannelPackageStateChecker = (params: { cfg: OpenClawConfig; @@ -34,7 +29,6 @@ type ChannelPackageStateRegistry = { }; const log = createSubsystemLogger("channels"); -const nodeRequire = createRequire(import.meta.url); const registryCache = new Map(); function resolveChannelPackageStateMetadata( @@ -53,32 +47,6 @@ function resolveChannelPackageStateMetadata( return { specifier, exportName }; } -function createModuleLoader() { - const jitiLoaders = new Map>(); - - return (modulePath: string) => { - const tryNative = - shouldPreferNativeJiti(modulePath) || modulePath.includes(`${path.sep}dist${path.sep}`); - const aliasMap = buildPluginLoaderAliasMap(modulePath, process.argv[1], import.meta.url); - const cacheKey = JSON.stringify({ - tryNative, - aliasMap: Object.entries(aliasMap).toSorted(([left], [right]) => left.localeCompare(right)), - }); - const cached = jitiLoaders.get(cacheKey); - if (cached) { - return cached; - } - const loader = createJiti(import.meta.url, { - ...buildPluginLoaderJitiOptions(aliasMap), - tryNative, - }); - jitiLoaders.set(cacheKey, loader); - return loader; - }; -} - -const loadModule = createModuleLoader(); - function getChannelPackageStateRegistry( metadataKey: ChannelPackageStateMetadataKey, ): ChannelPackageStateRegistry { @@ -98,57 +66,6 @@ function getChannelPackageStateRegistry( return registry; } -function resolveModuleCandidates(entry: PluginChannelCatalogEntry, specifier: string): string[] { - const normalizedSpecifier = specifier.replace(/\\/g, "/"); - const resolvedPath = path.resolve(entry.rootDir, normalizedSpecifier); - const ext = path.extname(resolvedPath); - if (ext) { - return [resolvedPath]; - } - return [ - resolvedPath, - `${resolvedPath}.ts`, - `${resolvedPath}.js`, - `${resolvedPath}.mjs`, - `${resolvedPath}.cjs`, - ]; -} - -function resolveExistingModulePath(entry: PluginChannelCatalogEntry, specifier: string): string { - for (const candidate of resolveModuleCandidates(entry, specifier)) { - if (fs.existsSync(candidate)) { - return candidate; - } - } - return path.resolve(entry.rootDir, specifier); -} - -function loadChannelPackageStateModule(modulePath: string, rootDir: string): unknown { - const opened = openBoundaryFileSync({ - absolutePath: modulePath, - rootPath: rootDir, - boundaryLabel: "plugin root", - rejectHardlinks: false, - skipLexicalRootCheck: true, - }); - if (!opened.ok) { - throw new Error("plugin package-state module escapes plugin root or fails alias checks"); - } - const safePath = opened.path; - fs.closeSync(opened.fd); - if ( - process.platform === "win32" && - [".js", ".mjs", ".cjs"].includes(path.extname(safePath).toLowerCase()) - ) { - try { - return nodeRequire(safePath); - } catch { - // Fall back to Jiti when native require cannot load the target. - } - } - return loadModule(safePath)(safePath); -} - function resolveChannelPackageStateChecker(params: { entry: PluginChannelCatalogEntry; metadataKey: ChannelPackageStateMetadataKey; @@ -166,10 +83,11 @@ function resolveChannelPackageStateChecker(params: { } try { - const moduleExport = loadChannelPackageStateModule( - resolveExistingModulePath(params.entry, metadata.specifier!), - params.entry.rootDir, - ) as Record; + const moduleExport = loadChannelPluginModule({ + modulePath: resolveExistingPluginModulePath(params.entry.rootDir, metadata.specifier!), + rootDir: params.entry.rootDir, + shouldTryNativeRequire: isJavaScriptModulePath, + }) as Record; const checker = moduleExport[metadata.exportName!] as ChannelPackageStateChecker | undefined; if (typeof checker !== "function") { throw new Error(`missing ${params.metadataKey} export ${metadata.exportName}`);