-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Add support for npm packages and .env files to custom tools #10336
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Review completed. Found 1 issue worth noting.
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
| private copyEnvFiles(toolDir: 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(this.cacheDir, 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 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)}`, | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When multiple tools have .env files, they will overwrite each other in the shared cache directory. If tools A and B each have different .env files with different secrets, loading tool B will overwrite tool A's .env, causing tool A to potentially read incorrect environment variables. Consider using tool-specific subdirectories (e.g., path.join(this.cacheDir, hash)) or documenting that users should use distinct env var names across tools.
Fix it with Roo Code or mention @roomote and request a fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a really good point. I'll follow up with a fix for this.
|
Addresses part of #10323 |
You can now have a custom tool that looks like:
Where your sensitive env vars (like
AI_GATEWAY_API_KEY) are stored in a.envfile in the same directory as the custom tool.