Просмотр исходного кода

Add support for npm packages and .env files to custom tools (#10336)

Co-authored-by: Roo Code <[email protected]>
Chris Estreich 3 дней назад
Родитель
Сommit
0d50ed649f

+ 98 - 1
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*=/)
+	})
 })

+ 72 - 10
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<Record<string, CustomToolDefinition>> {
 		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.

+ 34 - 0
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 }