Skip to content

Commit 0d50ed6

Browse files
cteroomote
andauthored
Add support for npm packages and .env files to custom tools (#10336)
Co-authored-by: Roo Code <[email protected]>
1 parent 343d5e9 commit 0d50ed6

File tree

3 files changed

+204
-11
lines changed

3 files changed

+204
-11
lines changed

packages/core/src/custom-tools/__tests__/esbuild-runner.spec.ts

Lines changed: 98 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import fs from "fs"
22
import os from "os"
33
import path from "path"
44

5-
import { getEsbuildScriptPath, runEsbuild } from "../esbuild-runner.js"
5+
import { getEsbuildScriptPath, runEsbuild, NODE_BUILTIN_MODULES, COMMONJS_REQUIRE_BANNER } from "../esbuild-runner.js"
66

77
describe("getEsbuildScriptPath", () => {
88
it("should find esbuild-wasm script in node_modules in development", () => {
@@ -153,4 +153,101 @@ describe("runEsbuild", () => {
153153
// File should be created successfully.
154154
expect(fs.existsSync(outputFile)).toBe(true)
155155
}, 30000)
156+
157+
it("should keep external modules as imports instead of bundling", async () => {
158+
const inputFile = path.join(tempDir, "input.ts")
159+
const outputFile = path.join(tempDir, "output.mjs")
160+
161+
// Write code that imports fs (a Node.js built-in).
162+
fs.writeFileSync(
163+
inputFile,
164+
`
165+
import fs from "fs"
166+
export function fileExists(p: string): boolean {
167+
return fs.existsSync(p)
168+
}
169+
`,
170+
)
171+
172+
await runEsbuild({
173+
entryPoint: inputFile,
174+
outfile: outputFile,
175+
format: "esm",
176+
bundle: true,
177+
external: ["fs"],
178+
})
179+
180+
const outputContent = fs.readFileSync(outputFile, "utf-8")
181+
// fs should remain as an import, not bundled.
182+
expect(outputContent).toMatch(/import.*from\s*["']fs["']/)
183+
}, 30000)
184+
185+
it("should add banner code when specified", async () => {
186+
const inputFile = path.join(tempDir, "input.ts")
187+
const outputFile = path.join(tempDir, "output.mjs")
188+
189+
fs.writeFileSync(inputFile, `export const greeting = "Hello"`)
190+
191+
const customBanner = "// This is a custom banner comment"
192+
await runEsbuild({
193+
entryPoint: inputFile,
194+
outfile: outputFile,
195+
format: "esm",
196+
banner: customBanner,
197+
})
198+
199+
const outputContent = fs.readFileSync(outputFile, "utf-8")
200+
// Banner should be at the start of the file.
201+
expect(outputContent.startsWith(customBanner)).toBe(true)
202+
}, 30000)
203+
204+
it("should add CommonJS require shim banner for ESM bundles", async () => {
205+
const inputFile = path.join(tempDir, "input.ts")
206+
const outputFile = path.join(tempDir, "output.mjs")
207+
208+
fs.writeFileSync(inputFile, `export const value = 42`)
209+
210+
await runEsbuild({
211+
entryPoint: inputFile,
212+
outfile: outputFile,
213+
format: "esm",
214+
banner: COMMONJS_REQUIRE_BANNER,
215+
})
216+
217+
const outputContent = fs.readFileSync(outputFile, "utf-8")
218+
// Should contain the createRequire shim.
219+
expect(outputContent).toContain("createRequire")
220+
expect(outputContent).toContain("import.meta.url")
221+
}, 30000)
222+
})
223+
224+
describe("NODE_BUILTIN_MODULES", () => {
225+
it("should include common Node.js built-in modules", () => {
226+
expect(NODE_BUILTIN_MODULES).toContain("fs")
227+
expect(NODE_BUILTIN_MODULES).toContain("path")
228+
expect(NODE_BUILTIN_MODULES).toContain("crypto")
229+
expect(NODE_BUILTIN_MODULES).toContain("http")
230+
expect(NODE_BUILTIN_MODULES).toContain("https")
231+
expect(NODE_BUILTIN_MODULES).toContain("os")
232+
expect(NODE_BUILTIN_MODULES).toContain("child_process")
233+
expect(NODE_BUILTIN_MODULES).toContain("stream")
234+
expect(NODE_BUILTIN_MODULES).toContain("util")
235+
expect(NODE_BUILTIN_MODULES).toContain("events")
236+
})
237+
238+
it("should be an array of strings", () => {
239+
expect(Array.isArray(NODE_BUILTIN_MODULES)).toBe(true)
240+
expect(NODE_BUILTIN_MODULES.every((m) => typeof m === "string")).toBe(true)
241+
})
242+
})
243+
244+
describe("COMMONJS_REQUIRE_BANNER", () => {
245+
it("should provide createRequire shim", () => {
246+
expect(COMMONJS_REQUIRE_BANNER).toContain("createRequire")
247+
expect(COMMONJS_REQUIRE_BANNER).toContain("import.meta.url")
248+
})
249+
250+
it("should define require variable", () => {
251+
expect(COMMONJS_REQUIRE_BANNER).toMatch(/var require\s*=/)
252+
})
156253
})

packages/core/src/custom-tools/custom-tool-registry.ts

Lines changed: 72 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import type { CustomToolDefinition, SerializedCustomToolDefinition, CustomToolPa
1717

1818
import type { StoredCustomTool, LoadResult } from "./types.js"
1919
import { serializeCustomTool } from "./serialize.js"
20-
import { runEsbuild } from "./esbuild-runner.js"
20+
import { runEsbuild, NODE_BUILTIN_MODULES, COMMONJS_REQUIRE_BANNER } from "./esbuild-runner.js"
2121

2222
export interface RegistryOptions {
2323
/** Directory for caching compiled TypeScript files. */
@@ -236,16 +236,22 @@ export class CustomToolRegistry {
236236

237237
/**
238238
* Clear the TypeScript compilation cache (both in-memory and on disk).
239+
* This removes all tool-specific subdirectories and their contents.
239240
*/
240241
clearCache(): void {
241242
this.tsCache.clear()
242243

243244
if (fs.existsSync(this.cacheDir)) {
244245
try {
245-
const files = fs.readdirSync(this.cacheDir)
246-
for (const file of files) {
247-
if (file.endsWith(".mjs")) {
248-
fs.unlinkSync(path.join(this.cacheDir, file))
246+
const entries = fs.readdirSync(this.cacheDir, { withFileTypes: true })
247+
for (const entry of entries) {
248+
const entryPath = path.join(this.cacheDir, entry.name)
249+
if (entry.isDirectory()) {
250+
// Remove tool-specific subdirectory and all its contents.
251+
fs.rmSync(entryPath, { recursive: true, force: true })
252+
} else if (entry.name.endsWith(".mjs")) {
253+
// Also clean up any legacy flat .mjs files from older cache format.
254+
fs.unlinkSync(entryPath)
249255
}
250256
}
251257
} catch (error) {
@@ -259,6 +265,11 @@ export class CustomToolRegistry {
259265
/**
260266
* Dynamically import a TypeScript or JavaScript file.
261267
* TypeScript files are transpiled on-the-fly using esbuild.
268+
*
269+
* For TypeScript files, esbuild bundles the code with these considerations:
270+
* - Node.js built-in modules (fs, path, etc.) are kept external
271+
* - npm packages are bundled with a CommonJS shim for require() compatibility
272+
* - The tool's local node_modules is included in the resolution path
262273
*/
263274
private async import(filePath: string): Promise<Record<string, CustomToolDefinition>> {
264275
const absolutePath = path.resolve(filePath)
@@ -277,19 +288,31 @@ export class CustomToolRegistry {
277288
return import(`file://${cachedPath}`)
278289
}
279290

280-
// Ensure cache directory exists.
281-
fs.mkdirSync(this.cacheDir, { recursive: true })
282-
283291
const hash = createHash("sha256").update(cacheKey).digest("hex").slice(0, 16)
284-
const tempFile = path.join(this.cacheDir, `${hash}.mjs`)
292+
293+
// Use a tool-specific subdirectory to avoid .env file conflicts between tools.
294+
const toolCacheDir = path.join(this.cacheDir, hash)
295+
fs.mkdirSync(toolCacheDir, { recursive: true })
296+
297+
const tempFile = path.join(toolCacheDir, "bundle.mjs")
285298

286299
// Check if we have a cached version on disk (from a previous run/instance).
287300
if (fs.existsSync(tempFile)) {
288301
this.tsCache.set(cacheKey, tempFile)
289302
return import(`file://${tempFile}`)
290303
}
291304

305+
// Get the tool's directory to include its node_modules in resolution path.
306+
const toolDir = path.dirname(absolutePath)
307+
const toolNodeModules = path.join(toolDir, "node_modules")
308+
309+
// Combine default nodePaths with tool-specific node_modules.
310+
// Tool's node_modules takes priority (listed first).
311+
const nodePaths = fs.existsSync(toolNodeModules) ? [toolNodeModules, ...this.nodePaths] : this.nodePaths
312+
292313
// Bundle the TypeScript file with dependencies using esbuild CLI.
314+
// - Node.js built-ins are external (they can't be bundled and are always available)
315+
// - npm packages are bundled with CommonJS require() shim for compatibility
293316
await runEsbuild(
294317
{
295318
entryPoint: absolutePath,
@@ -300,15 +323,54 @@ export class CustomToolRegistry {
300323
bundle: true,
301324
sourcemap: "inline",
302325
packages: "bundle",
303-
nodePaths: this.nodePaths,
326+
nodePaths,
327+
external: NODE_BUILTIN_MODULES,
328+
banner: COMMONJS_REQUIRE_BANNER,
304329
},
305330
this.extensionPath,
306331
)
307332

333+
// Copy .env files from the tool's source directory to the tool-specific cache directory.
334+
// This allows tools that use dotenv with __dirname to find their .env files,
335+
// while ensuring different tools' .env files don't overwrite each other.
336+
this.copyEnvFiles(toolDir, toolCacheDir)
337+
308338
this.tsCache.set(cacheKey, tempFile)
309339
return import(`file://${tempFile}`)
310340
}
311341

342+
/**
343+
* Copy .env files from the tool's source directory to the tool-specific cache directory.
344+
* This allows tools that use dotenv with __dirname to find their .env files,
345+
* while ensuring different tools' .env files don't overwrite each other.
346+
*
347+
* @param toolDir - The directory containing the tool source files
348+
* @param destDir - The tool-specific cache directory to copy .env files to
349+
*/
350+
private copyEnvFiles(toolDir: string, destDir: string): void {
351+
try {
352+
const files = fs.readdirSync(toolDir)
353+
const envFiles = files.filter((f) => f === ".env" || f.startsWith(".env."))
354+
355+
for (const envFile of envFiles) {
356+
const srcPath = path.join(toolDir, envFile)
357+
const destPath = path.join(destDir, envFile)
358+
359+
// Only copy if source is a file (not a directory).
360+
const stat = fs.statSync(srcPath)
361+
if (stat.isFile()) {
362+
fs.copyFileSync(srcPath, destPath)
363+
console.log(`[CustomToolRegistry] copied ${envFile} to tool cache directory`)
364+
}
365+
}
366+
} catch (error) {
367+
// Non-fatal: log but don't fail if we can't copy env files.
368+
console.warn(
369+
`[CustomToolRegistry] failed to copy .env files: ${error instanceof Error ? error.message : String(error)}`,
370+
)
371+
}
372+
}
373+
312374
/**
313375
* Check if a value is a Zod schema by looking for the _def property
314376
* which is present on all Zod types.

packages/core/src/custom-tools/esbuild-runner.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,27 @@
1111

1212
import path from "path"
1313
import fs from "fs"
14+
import { builtinModules } from "module"
1415
import { fileURLToPath } from "url"
1516
import { execa } from "execa"
1617

18+
/**
19+
* Node.js built-in modules that should never be bundled.
20+
* These are always available in Node.js runtime and bundling them causes issues.
21+
*
22+
* Uses Node.js's authoritative list from `module.builtinModules` and adds
23+
* the `node:` prefixed versions for comprehensive coverage.
24+
*/
25+
export const NODE_BUILTIN_MODULES: readonly string[] = [...builtinModules, ...builtinModules.map((m) => `node:${m}`)]
26+
27+
/**
28+
* Banner code to add to bundled output.
29+
* This provides a CommonJS-compatible `require` function for ESM bundles,
30+
* which is needed when bundled npm packages use `require()` internally.
31+
*/
32+
export const COMMONJS_REQUIRE_BANNER = `import { createRequire as __roo_createRequire } from 'module';
33+
var require = __roo_createRequire(import.meta.url);`
34+
1735
// Get the directory where this module is located.
1836
function getModuleDir(): string | undefined {
1937
try {
@@ -50,6 +68,10 @@ export interface EsbuildOptions {
5068
packages?: "bundle" | "external"
5169
/** Additional paths for module resolution */
5270
nodePaths?: string[]
71+
/** Modules to exclude from bundling (resolved at runtime) */
72+
external?: readonly string[]
73+
/** JavaScript code to prepend to the output bundle */
74+
banner?: string
5375
}
5476

5577
/**
@@ -158,6 +180,18 @@ export async function runEsbuild(options: EsbuildOptions, extensionPath?: string
158180
args.push(`--packages=${options.packages}`)
159181
}
160182

183+
// Add external modules - these won't be bundled and will be resolved at runtime.
184+
if (options.external && options.external.length > 0) {
185+
for (const ext of options.external) {
186+
args.push(`--external:${ext}`)
187+
}
188+
}
189+
190+
// Add banner code (e.g., for CommonJS require shim in ESM bundles).
191+
if (options.banner) {
192+
args.push(`--banner:js=${options.banner}`)
193+
}
194+
161195
// Build environment with NODE_PATH for module resolution.
162196
const env: NodeJS.ProcessEnv = { ...process.env }
163197

0 commit comments

Comments
 (0)