Skip to content

Commit 42773cb

Browse files
steipetesteipete
committed
refactor(channels): load bundled modules without jiti
1 parent 890a053 commit 42773cb

5 files changed

Lines changed: 87 additions & 96 deletions

File tree

‎src/channels/plugins/bundled.shape-guard.test.ts‎

Lines changed: 45 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,8 @@ function listSourceBundledPluginRoots(): string[] {
105105
}
106106

107107
afterEach(() => {
108+
delete (globalThis as { __openclawBundledChannelReenter?: () => void })
109+
.__openclawBundledChannelReenter;
108110
vi.resetModules();
109111
vi.doUnmock("../../plugins/bundled-channel-runtime.js");
110112
vi.doUnmock("../../plugins/bundled-plugin-metadata.js");
@@ -835,29 +837,56 @@ describe("bundled channel entry shape guards", () => {
835837

836838
it("breaks reentrant bundled channel discovery cycles with an empty fallback", async () => {
837839
const pluginDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-bundled-reentrant-"));
838-
const modulePath = path.join(pluginDir, "index.js");
839-
fs.writeFileSync(modulePath, "export {};\n", "utf8");
840+
const modulePath = path.join(pluginDir, "index.cjs");
841+
fs.writeFileSync(
842+
modulePath,
843+
`
844+
const reenter = globalThis.__openclawBundledChannelReenter;
845+
if (typeof reenter === "function") {
846+
reenter();
847+
}
848+
module.exports = {
849+
default: {
850+
kind: "bundled-channel-entry",
851+
id: "alpha",
852+
name: "Alpha",
853+
description: "Alpha",
854+
configSchema: {},
855+
register() {},
856+
loadChannelPlugin() {
857+
return {
858+
id: "alpha",
859+
meta: {},
860+
capabilities: {},
861+
config: {},
862+
};
863+
},
864+
},
865+
};
866+
`,
867+
"utf8",
868+
);
840869

841-
vi.doMock("../../plugins/bundled-plugin-metadata.js", async (importOriginal) => {
870+
vi.doMock("../../plugins/bundled-channel-runtime.js", async (importOriginal) => {
842871
const actual =
843-
await importOriginal<typeof import("../../plugins/bundled-plugin-metadata.js")>();
872+
await importOriginal<typeof import("../../plugins/bundled-channel-runtime.js")>();
844873
return {
845874
...actual,
846-
listBundledPluginMetadata: () => [
875+
listBundledChannelPluginMetadata: () => [
847876
{
848877
dirName: "alpha",
849878
idHint: "alpha",
850879
source: {
851-
source: "./index.js",
852-
built: "./index.js",
880+
source: "./index.cjs",
881+
built: "./index.cjs",
853882
},
854883
manifest: {
855884
id: "alpha",
856885
channels: ["alpha"],
857886
},
858887
},
859888
],
860-
resolveBundledPluginGeneratedPath: () => modulePath,
889+
resolveBundledChannelGeneratedPath: () => modulePath,
861890
};
862891
});
863892
vi.doMock("../../infra/boundary-file-read.js", () => ({
@@ -870,44 +899,16 @@ describe("bundled channel entry shape guards", () => {
870899
vi.doMock("../../plugins/channel-catalog-registry.js", () => ({
871900
listChannelCatalogEntries: () => [],
872901
}));
873-
// jiti-loader-cache prefers native require() for compiled .js before
874-
// falling back to jiti. This test drives plugin loading via the jiti
875-
// mock — disable the native-require fast path so the mocked jiti loader
876-
// is exercised instead of loading the on-disk fixture directly.
877-
vi.doMock("../../plugins/native-module-require.js", () => ({
878-
isJavaScriptModulePath: () => false,
879-
tryNativeRequireJavaScriptModule: () => ({ ok: false }),
880-
}));
881902

882903
let reentered = false;
883-
vi.doMock("jiti", () => ({
884-
createJiti: () => {
885-
return () => {
886-
if (!reentered) {
887-
reentered = true;
888-
expect(bundled.listBundledChannelPlugins()).toEqual([]);
889-
}
890-
return {
891-
default: {
892-
kind: "bundled-channel-entry",
893-
id: "alpha",
894-
name: "Alpha",
895-
description: "Alpha",
896-
configSchema: {},
897-
register() {},
898-
loadChannelPlugin() {
899-
return {
900-
id: "alpha",
901-
meta: {},
902-
capabilities: {},
903-
config: {},
904-
};
905-
},
906-
},
907-
};
908-
};
909-
},
910-
}));
904+
(
905+
globalThis as { __openclawBundledChannelReenter?: () => void }
906+
).__openclawBundledChannelReenter = () => {
907+
if (!reentered) {
908+
reentered = true;
909+
expect(bundled.listBundledChannelPlugins()).toEqual([]);
910+
}
911+
};
911912

912913
const bundled = await importFreshModule<typeof import("./bundled.js")>(
913914
import.meta.url,

‎src/channels/plugins/bundled.ts‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import {
1515
import { normalizePluginsConfig } from "../../plugins/config-state.js";
1616
import { passesManifestOwnerBasePolicy } from "../../plugins/manifest-owner-policy.js";
1717
import { unwrapDefaultModuleExport } from "../../plugins/module-export.js";
18-
import { isJavaScriptModulePath } from "../../plugins/native-module-require.js";
1918
import type { PluginRuntime } from "../../plugins/runtime/types.js";
2019
import { normalizeOptionalLowercaseString } from "../../shared/string-coerce.js";
2120
import { resolveBundledChannelRootScope, type BundledChannelRootScope } from "./bundled-root.js";
@@ -209,8 +208,6 @@ function loadGeneratedBundledChannelModule(params: {
209208
modulePath,
210209
rootDir: boundaryRoot,
211210
boundaryRootDir: boundaryRoot,
212-
shouldTryNativeRequire: (safePath) =>
213-
safePath.includes(`${path.sep}dist${path.sep}`) && isJavaScriptModulePath(safePath),
214211
});
215212
}
216213

‎src/channels/plugins/module-loader.test.ts‎

Lines changed: 18 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ afterEach(() => {
1313
fs.rmSync(tempDir, { recursive: true, force: true });
1414
}
1515
vi.resetModules();
16-
vi.doUnmock("jiti");
1716
});
1817

1918
function createTempDir(): string {
@@ -38,7 +37,7 @@ describe("channel plugin module loader helpers", () => {
3837
expect(isJavaScriptModulePath("/tmp/entry.ts")).toBe(false);
3938
});
4039

41-
it("uses native require for eligible JavaScript modules before falling back to Jiti", async () => {
40+
it("uses native require for eligible JavaScript modules without creating Jiti", async () => {
4241
const createJiti = vi.fn(() => vi.fn(() => ({ ok: false })));
4342
vi.resetModules();
4443
vi.doMock("jiti", () => ({
@@ -57,45 +56,34 @@ describe("channel plugin module loader helpers", () => {
5756
loaderModule.loadChannelPluginModule({
5857
modulePath,
5958
rootDir,
60-
shouldTryNativeRequire: () => true,
6159
}),
6260
).toEqual({ ok: true });
6361
expect(createJiti).not.toHaveBeenCalled();
6462
});
6563

66-
it("creates the runtime-supported Jiti boundary for Windows dist loads", async () => {
67-
const createJiti = vi.fn(() => vi.fn(() => ({ ok: true })));
64+
it("rejects TypeScript modules without creating Jiti", async () => {
65+
const createJiti = vi.fn(() => {
66+
throw new Error("channel module loader must not create jiti");
67+
});
6868
vi.resetModules();
6969
vi.doMock("jiti", () => ({
7070
createJiti,
7171
}));
72-
const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32");
73-
74-
try {
75-
const loaderModule = await importFreshModule<typeof import("./module-loader.js")>(
76-
import.meta.url,
77-
"./module-loader.js?scope=windows-dist-jiti",
78-
);
79-
const rootDir = createTempDir();
80-
const modulePath = path.join(rootDir, "dist", "extensions", "demo", "index.js");
81-
fs.mkdirSync(path.dirname(modulePath), { recursive: true });
82-
fs.writeFileSync(modulePath, "export const ok = true;\n", "utf8");
72+
const loaderModule = await importFreshModule<typeof import("./module-loader.js")>(
73+
import.meta.url,
74+
"./module-loader.js?scope=source-ts-native-hook",
75+
);
76+
const rootDir = createTempDir();
77+
const modulePath = path.join(rootDir, "extensions", "demo", "index.ts");
78+
fs.mkdirSync(path.dirname(modulePath), { recursive: true });
79+
fs.writeFileSync(modulePath, "export const ok = true;\n", "utf8");
8380

84-
const loaded = loaderModule.loadChannelPluginModule({
81+
expect(() =>
82+
loaderModule.loadChannelPluginModule({
8583
modulePath,
8684
rootDir,
87-
shouldTryNativeRequire: () => false,
88-
});
89-
90-
expect(loaded).toMatchObject({ ok: true });
91-
expect(createJiti).toHaveBeenCalledWith(
92-
expect.any(String),
93-
expect.objectContaining({
94-
tryNative: false,
95-
}),
96-
);
97-
} finally {
98-
platformSpy.mockRestore();
99-
}
85+
}),
86+
).toThrow(/must be built JavaScript/u);
87+
expect(createJiti).not.toHaveBeenCalled();
10088
});
10189
});

‎src/channels/plugins/module-loader.ts‎

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,31 @@
11
import fs from "node:fs";
2+
import { createRequire } from "node:module";
23
import path from "node:path";
34
import { openBoundaryFileSync } from "../../infra/boundary-file-read.js";
4-
import {
5-
getCachedPluginJitiLoader,
6-
type PluginJitiLoaderCache,
7-
} from "../../plugins/jiti-loader-cache.js";
5+
import { isJavaScriptModulePath } from "../../plugins/native-module-require.js";
86

9-
const jitiLoaders: PluginJitiLoaderCache = new Map();
7+
const nodeRequire = createRequire(import.meta.url);
8+
const SOURCE_MODULE_EXTENSIONS = new Set([".ts", ".tsx", ".mts", ".cts"]);
109

11-
function loadModule(modulePath: string, tryNative?: boolean) {
12-
return getCachedPluginJitiLoader({
13-
cache: jitiLoaders,
14-
modulePath,
15-
importerUrl: import.meta.url,
16-
argvEntry: process.argv[1],
17-
preferBuiltDist: true,
18-
jitiFilename: import.meta.url,
19-
tryNative,
20-
});
10+
function hasNativeSourceRequireHook(modulePath: string): boolean {
11+
const extension = path.extname(modulePath).toLowerCase();
12+
return (
13+
SOURCE_MODULE_EXTENSIONS.has(extension) &&
14+
typeof nodeRequire.extensions?.[extension] === "function"
15+
);
16+
}
17+
18+
function loadModule(modulePath: string): unknown {
19+
if (!isJavaScriptModulePath(modulePath) && !hasNativeSourceRequireHook(modulePath)) {
20+
throw new Error(`channel plugin module must be built JavaScript: ${modulePath}`);
21+
}
22+
try {
23+
return nodeRequire(modulePath);
24+
} catch (error) {
25+
throw new Error(`failed to load channel plugin module with native require: ${modulePath}`, {
26+
cause: error,
27+
});
28+
}
2129
}
2230

2331
function resolvePluginModuleCandidates(rootDir: string, specifier: string): string[] {
@@ -52,7 +60,6 @@ export function loadChannelPluginModule(params: {
5260
rootDir: string;
5361
boundaryRootDir?: string;
5462
boundaryLabel?: string;
55-
shouldTryNativeRequire?: (safePath: string) => boolean;
5663
}): unknown {
5764
const opened = openBoundaryFileSync({
5865
absolutePath: params.modulePath,
@@ -68,5 +75,5 @@ export function loadChannelPluginModule(params: {
6875
}
6976
const safePath = opened.path;
7077
fs.closeSync(opened.fd);
71-
return loadModule(safePath, params.shouldTryNativeRequire?.(safePath))(safePath);
78+
return loadModule(safePath);
7279
}

‎src/channels/plugins/package-state-probes.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import {
55
listChannelCatalogEntries,
66
type PluginChannelCatalogEntry,
77
} from "../../plugins/channel-catalog-registry.js";
8-
import { isJavaScriptModulePath } from "../../plugins/native-module-require.js";
98
import { normalizeOptionalString } from "../../shared/string-coerce.js";
109
import { loadChannelPluginModule, resolveExistingPluginModulePath } from "./module-loader.js";
1110

@@ -60,7 +59,6 @@ function resolveChannelPackageStateChecker(params: {
6059
const moduleExport = loadChannelPluginModule({
6160
modulePath: resolveExistingPluginModulePath(params.entry.rootDir, metadata.specifier!),
6261
rootDir: params.entry.rootDir,
63-
shouldTryNativeRequire: isJavaScriptModulePath,
6462
}) as Record<string, unknown>;
6563
const checker = moduleExport[metadata.exportName!] as ChannelPackageStateChecker | undefined;
6664
if (typeof checker !== "function") {

0 commit comments

Comments
 (0)