diff --git a/packages/core/src/custom-tools/__tests__/esbuild-runner.spec.ts b/packages/core/src/custom-tools/__tests__/esbuild-runner.spec.ts index 78581fc7c5..affb1a3734 100644 --- a/packages/core/src/custom-tools/__tests__/esbuild-runner.spec.ts +++ b/packages/core/src/custom-tools/__tests__/esbuild-runner.spec.ts @@ -2,7 +2,7 @@ import fs from "fs" import os from "os" import path from "path" -import { getEsbuildScriptPath, runEsbuild } from "../esbuild-runner.js" +import { getEsbuildScriptPath, runEsbuild, NODE_BUILTIN_MODULES, COMMONJS_REQUIRE_BANNER } from "../esbuild-runner.js" describe("getEsbuildScriptPath", () => { it("should find esbuild-wasm script in node_modules in development", () => { @@ -153,4 +153,101 @@ describe("runEsbuild", () => { // File should be created successfully. expect(fs.existsSync(outputFile)).toBe(true) }, 30000) + + it("should keep external modules as imports instead of bundling", async () => { + const inputFile = path.join(tempDir, "input.ts") + const outputFile = path.join(tempDir, "output.mjs") + + // Write code that imports fs (a Node.js built-in). + fs.writeFileSync( + inputFile, + ` + import fs from "fs" + export function fileExists(p: string): boolean { + return fs.existsSync(p) + } + `, + ) + + await runEsbuild({ + entryPoint: inputFile, + outfile: outputFile, + format: "esm", + bundle: true, + external: ["fs"], + }) + + const outputContent = fs.readFileSync(outputFile, "utf-8") + // fs should remain as an import, not bundled. + expect(outputContent).toMatch(/import.*from\s*["']fs["']/) + }, 30000) + + it("should add banner code when specified", async () => { + const inputFile = path.join(tempDir, "input.ts") + const outputFile = path.join(tempDir, "output.mjs") + + fs.writeFileSync(inputFile, `export const greeting = "Hello"`) + + const customBanner = "// This is a custom banner comment" + await runEsbuild({ + entryPoint: inputFile, + outfile: outputFile, + format: "esm", + banner: customBanner, + }) + + const outputContent = fs.readFileSync(outputFile, "utf-8") + // Banner should be at the start of the file. + expect(outputContent.startsWith(customBanner)).toBe(true) + }, 30000) + + it("should add CommonJS require shim banner for ESM bundles", async () => { + const inputFile = path.join(tempDir, "input.ts") + const outputFile = path.join(tempDir, "output.mjs") + + fs.writeFileSync(inputFile, `export const value = 42`) + + await runEsbuild({ + entryPoint: inputFile, + outfile: outputFile, + format: "esm", + banner: COMMONJS_REQUIRE_BANNER, + }) + + const outputContent = fs.readFileSync(outputFile, "utf-8") + // Should contain the createRequire shim. + expect(outputContent).toContain("createRequire") + expect(outputContent).toContain("import.meta.url") + }, 30000) +}) + +describe("NODE_BUILTIN_MODULES", () => { + it("should include common Node.js built-in modules", () => { + expect(NODE_BUILTIN_MODULES).toContain("fs") + expect(NODE_BUILTIN_MODULES).toContain("path") + expect(NODE_BUILTIN_MODULES).toContain("crypto") + expect(NODE_BUILTIN_MODULES).toContain("http") + expect(NODE_BUILTIN_MODULES).toContain("https") + expect(NODE_BUILTIN_MODULES).toContain("os") + expect(NODE_BUILTIN_MODULES).toContain("child_process") + expect(NODE_BUILTIN_MODULES).toContain("stream") + expect(NODE_BUILTIN_MODULES).toContain("util") + expect(NODE_BUILTIN_MODULES).toContain("events") + }) + + it("should be an array of strings", () => { + expect(Array.isArray(NODE_BUILTIN_MODULES)).toBe(true) + expect(NODE_BUILTIN_MODULES.every((m) => typeof m === "string")).toBe(true) + }) +}) + +describe("COMMONJS_REQUIRE_BANNER", () => { + it("should provide createRequire shim", () => { + expect(COMMONJS_REQUIRE_BANNER).toContain("createRequire") + expect(COMMONJS_REQUIRE_BANNER).toContain("import.meta.url") + }) + + it("should define require variable", () => { + expect(COMMONJS_REQUIRE_BANNER).toMatch(/var require\s*=/) + }) }) diff --git a/packages/core/src/custom-tools/custom-tool-registry.ts b/packages/core/src/custom-tools/custom-tool-registry.ts index ee72f68d8e..1725f4aba3 100644 --- a/packages/core/src/custom-tools/custom-tool-registry.ts +++ b/packages/core/src/custom-tools/custom-tool-registry.ts @@ -17,7 +17,7 @@ import type { CustomToolDefinition, SerializedCustomToolDefinition, CustomToolPa import type { StoredCustomTool, LoadResult } from "./types.js" import { serializeCustomTool } from "./serialize.js" -import { runEsbuild } from "./esbuild-runner.js" +import { runEsbuild, NODE_BUILTIN_MODULES, COMMONJS_REQUIRE_BANNER } from "./esbuild-runner.js" export interface RegistryOptions { /** Directory for caching compiled TypeScript files. */ @@ -236,16 +236,22 @@ export class CustomToolRegistry { /** * Clear the TypeScript compilation cache (both in-memory and on disk). + * This removes all tool-specific subdirectories and their contents. */ clearCache(): void { this.tsCache.clear() if (fs.existsSync(this.cacheDir)) { try { - const files = fs.readdirSync(this.cacheDir) - for (const file of files) { - if (file.endsWith(".mjs")) { - fs.unlinkSync(path.join(this.cacheDir, file)) + const entries = fs.readdirSync(this.cacheDir, { withFileTypes: true }) + for (const entry of entries) { + const entryPath = path.join(this.cacheDir, entry.name) + if (entry.isDirectory()) { + // Remove tool-specific subdirectory and all its contents. + fs.rmSync(entryPath, { recursive: true, force: true }) + } else if (entry.name.endsWith(".mjs")) { + // Also clean up any legacy flat .mjs files from older cache format. + fs.unlinkSync(entryPath) } } } catch (error) { @@ -259,6 +265,11 @@ export class CustomToolRegistry { /** * Dynamically import a TypeScript or JavaScript file. * TypeScript files are transpiled on-the-fly using esbuild. + * + * For TypeScript files, esbuild bundles the code with these considerations: + * - Node.js built-in modules (fs, path, etc.) are kept external + * - npm packages are bundled with a CommonJS shim for require() compatibility + * - The tool's local node_modules is included in the resolution path */ private async import(filePath: string): Promise> { const absolutePath = path.resolve(filePath) @@ -277,11 +288,13 @@ export class CustomToolRegistry { return import(`file://${cachedPath}`) } - // Ensure cache directory exists. - fs.mkdirSync(this.cacheDir, { recursive: true }) - const hash = createHash("sha256").update(cacheKey).digest("hex").slice(0, 16) - const tempFile = path.join(this.cacheDir, `${hash}.mjs`) + + // Use a tool-specific subdirectory to avoid .env file conflicts between tools. + const toolCacheDir = path.join(this.cacheDir, hash) + fs.mkdirSync(toolCacheDir, { recursive: true }) + + const tempFile = path.join(toolCacheDir, "bundle.mjs") // Check if we have a cached version on disk (from a previous run/instance). if (fs.existsSync(tempFile)) { @@ -289,7 +302,17 @@ export class CustomToolRegistry { return import(`file://${tempFile}`) } + // Get the tool's directory to include its node_modules in resolution path. + const toolDir = path.dirname(absolutePath) + const toolNodeModules = path.join(toolDir, "node_modules") + + // Combine default nodePaths with tool-specific node_modules. + // Tool's node_modules takes priority (listed first). + const nodePaths = fs.existsSync(toolNodeModules) ? [toolNodeModules, ...this.nodePaths] : this.nodePaths + // Bundle the TypeScript file with dependencies using esbuild CLI. + // - Node.js built-ins are external (they can't be bundled and are always available) + // - npm packages are bundled with CommonJS require() shim for compatibility await runEsbuild( { entryPoint: absolutePath, @@ -300,15 +323,54 @@ export class CustomToolRegistry { bundle: true, sourcemap: "inline", packages: "bundle", - nodePaths: this.nodePaths, + nodePaths, + external: NODE_BUILTIN_MODULES, + banner: COMMONJS_REQUIRE_BANNER, }, this.extensionPath, ) + // Copy .env files from the tool's source directory to the tool-specific cache directory. + // This allows tools that use dotenv with __dirname to find their .env files, + // while ensuring different tools' .env files don't overwrite each other. + this.copyEnvFiles(toolDir, toolCacheDir) + this.tsCache.set(cacheKey, tempFile) return import(`file://${tempFile}`) } + /** + * Copy .env files from the tool's source directory to the tool-specific cache directory. + * This allows tools that use dotenv with __dirname to find their .env files, + * while ensuring different tools' .env files don't overwrite each other. + * + * @param toolDir - The directory containing the tool source files + * @param destDir - The tool-specific cache directory to copy .env files to + */ + private copyEnvFiles(toolDir: string, destDir: string): void { + try { + const files = fs.readdirSync(toolDir) + const envFiles = files.filter((f) => f === ".env" || f.startsWith(".env.")) + + for (const envFile of envFiles) { + const srcPath = path.join(toolDir, envFile) + const destPath = path.join(destDir, envFile) + + // Only copy if source is a file (not a directory). + const stat = fs.statSync(srcPath) + if (stat.isFile()) { + fs.copyFileSync(srcPath, destPath) + console.log(`[CustomToolRegistry] copied ${envFile} to tool cache directory`) + } + } + } catch (error) { + // Non-fatal: log but don't fail if we can't copy env files. + console.warn( + `[CustomToolRegistry] failed to copy .env files: ${error instanceof Error ? error.message : String(error)}`, + ) + } + } + /** * Check if a value is a Zod schema by looking for the _def property * which is present on all Zod types. diff --git a/packages/core/src/custom-tools/esbuild-runner.ts b/packages/core/src/custom-tools/esbuild-runner.ts index 4138478921..6f62143551 100644 --- a/packages/core/src/custom-tools/esbuild-runner.ts +++ b/packages/core/src/custom-tools/esbuild-runner.ts @@ -11,9 +11,27 @@ import path from "path" import fs from "fs" +import { builtinModules } from "module" import { fileURLToPath } from "url" import { execa } from "execa" +/** + * Node.js built-in modules that should never be bundled. + * These are always available in Node.js runtime and bundling them causes issues. + * + * Uses Node.js's authoritative list from `module.builtinModules` and adds + * the `node:` prefixed versions for comprehensive coverage. + */ +export const NODE_BUILTIN_MODULES: readonly string[] = [...builtinModules, ...builtinModules.map((m) => `node:${m}`)] + +/** + * Banner code to add to bundled output. + * This provides a CommonJS-compatible `require` function for ESM bundles, + * which is needed when bundled npm packages use `require()` internally. + */ +export const COMMONJS_REQUIRE_BANNER = `import { createRequire as __roo_createRequire } from 'module'; +var require = __roo_createRequire(import.meta.url);` + // Get the directory where this module is located. function getModuleDir(): string | undefined { try { @@ -50,6 +68,10 @@ export interface EsbuildOptions { packages?: "bundle" | "external" /** Additional paths for module resolution */ nodePaths?: string[] + /** Modules to exclude from bundling (resolved at runtime) */ + external?: readonly string[] + /** JavaScript code to prepend to the output bundle */ + banner?: string } /** @@ -158,6 +180,18 @@ export async function runEsbuild(options: EsbuildOptions, extensionPath?: string args.push(`--packages=${options.packages}`) } + // Add external modules - these won't be bundled and will be resolved at runtime. + if (options.external && options.external.length > 0) { + for (const ext of options.external) { + args.push(`--external:${ext}`) + } + } + + // Add banner code (e.g., for CommonJS require shim in ESM bundles). + if (options.banner) { + args.push(`--banner:js=${options.banner}`) + } + // Build environment with NODE_PATH for module resolution. const env: NodeJS.ProcessEnv = { ...process.env }