-
Notifications
You must be signed in to change notification settings - Fork 1
[EVAL] API key from Env Var support #31
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
[EVAL] API key from Env Var support #31
Conversation
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
…ssage Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
…viders Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
OPENAI_API_KEY env var Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
whole process.env into webview Signed-off-by: Geoff Wilson <[email protected]>
of api key env var; use constants for env var keys. Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
errant keys in test file Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
vercel copy/pasta Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
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.
I've reviewed the PR and found several issues that need to be addressed:
-
Validation inconsistency: The validation logic in
validate.tswasn't updated for all providers that now support environment variables. This will cause validation errors when users try to use env vars for providers like deepinfra, huggingface, cerebras, fireworks, io-intelligence, featherless, vercel-ai-gateway, doubao, moonshot, sambanova, and zai. -
Typo in constant name:
MOONSHOOTshould beMOONSHOTin the constants file.
The core implementation looks solid - the ApiKey component is well-designed, the env var checking in ClineProvider is secure (only exposing boolean existence), and the backend integration properly reads from env vars when the flag is set. The test coverage is comprehensive.
Once the validation logic is updated to match all the new env var flags, this feature will work correctly across all providers.
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.
Several providers have env var support added in their UI components but are missing corresponding validation updates:
- deepinfra (line 61-65): Should check
deepInfraConfigUseEnvVars - huggingface (line 123-130): Should check
huggingFaceConfigUseEnvVars - cerebras (line 131-135): Should check
cerebrasConfigUseEnvVars - fireworks (line 136-140): Should check
fireworksConfigUseEnvVars - io-intelligence (line 141-145): Should check
ioIntelligenceConfigUseEnvVars - featherless (line 146-150): Should check
featherlessConfigUseEnvVars - vercel-ai-gateway (line 156-160): Should check
vercelConfigUseEnvVars - doubao, moonshot, sambanova, zai: No validation cases exist but env var support was added
Without these updates, users will get validation errors when trying to use environment variables for these providers even though the UI allows it.
| FIREWORKS: 'FIREWORKS_API_KEY', | ||
| HUGGING_FACE: 'HUGGINGFACE_API_KEY', | ||
| IO_INTELLIGENCE: 'IOINTELLIGENCE_API_KEY', | ||
| MOONSHOOT: 'MOONSHOT_API_KEY', |
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.
Typo in constant name: MOONSHOOT should be MOONSHOT to match the naming pattern of other providers and the actual environment variable name MOONSHOT_API_KEY.
| MOONSHOOT: 'MOONSHOT_API_KEY', | |
| MOONSHOT: 'MOONSHOT_API_KEY', |
| </div> | ||
| <ApiKey | ||
| apiKey={apiConfiguration?.moonshotApiKey || ""} | ||
| apiKeyEnvVar={API_KEYS.MOONSHOOT} |
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.
Using incorrect constant name MOONSHOOT instead of MOONSHOT. This typo is in the constants file (line 25 of packages/types/src/constants.ts) and needs to be fixed there.
| apiKeyEnvVar={API_KEYS.MOONSHOOT} | |
| apiKeyEnvVar={API_KEYS.MOONSHOT} |
| </div> | ||
| <ApiKey | ||
| apiKey={apiConfiguration?.moonshotApiKey || ""} | ||
| apiKeyEnvVar={API_KEYS.MOONSHOOT} |
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.
Using incorrect constant name MOONSHOOT instead of MOONSHOT. This typo is in the constants file (line 25 of packages/types/src/constants.ts) and needs to be fixed there.
| apiKeyEnvVar={API_KEYS.MOONSHOOT} | |
| apiKeyEnvVar={API_KEYS.MOONSHOT} |
🧪 Evaluation PR for Testing
This is an automated test PR created for evaluation purposes.
Related GitHub Issue
Closes: #4337
Description
This PR adds support for obtaining API keys from the environment. A checkbox is added to each configuration that allows inputting an API key -- when checked, a given environment variable (eg, OPENAI_API_KEY, etc) is consulted for the key and the user is not required to provide one in the configuration. Env Var key is predictable but not editable.
Test Procedure
I tested this using VS Code run configurations, where the expected environment variable could be provided or omitted.
Pre-Submission Checklist
A documentation update is advised, to describe environment variable usage to supply API keys.
Summary
Adds support for using environment variables for API keys across multiple providers, updates UI components to reflect this, and enhances testing and translation capabilities.