Added support to fetch credentials based on profile from existing Turbot CLI configuration. #1
Conversation
…in env.ts. Added support for reading credentials from a YAML file and improved error messaging for missing variables.
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for fetching credentials based on a profile from an existing Turbot CLI configuration.
- Introduces CLI-based credential lookup by reading a YAML file from a default or provided path.
- Adds logic to differentiate between direct environment variable credentials and CLI credentials.
Comments suppressed due to low confidence (2)
src/config/env.ts:9
- [nitpick] The variable name 'set1' is ambiguous. Consider renaming it to something more descriptive like 'directCredentialVars' to better convey its purpose.
const set1 = [
src/config/env.ts:15
- [nitpick] The variable name 'set2' is ambiguous. Consider renaming it to something like 'cliCredentialEnvVars' for clarity.
const set2 = [
| // Export validated environment variables | ||
| const config = { | ||
| TURBOT_GRAPHQL_ENDPOINT: process.env.TURBOT_GRAPHQL_ENDPOINT!, | ||
| TURBOT_ACCESS_KEY_ID: process.env.TURBOT_ACCESS_KEY_ID!, | ||
| TURBOT_SECRET_ACCESS_KEY: process.env.TURBOT_SECRET_ACCESS_KEY!, | ||
| let config: any = { |
There was a problem hiding this comment.
Avoid using 'any' for the config variable. Defining a proper type or interface would improve type safety and maintainability.
Copilot uses AI. Check for mistakes.
| if (endpoint.endsWith('/')) { | ||
| endpoint = endpoint.slice(0, -1); | ||
| } | ||
| endpoint = `${endpoint}/api/latest/graphql`; |
There was a problem hiding this comment.
Should it always add this to the endpoint? Or should we check for it first?
(This might be something to improve across both credential models!)
| hasSet2 = true; | ||
| } else { | ||
| throw new Error( | ||
| `❌ TURBOT_CLI_CREDENTIALS_PATH is required because the default credentials path could not be determined (HOME is not set).` |
There was a problem hiding this comment.
Let's remove the emoji's. They are not really required and smell like an LLM.
| // Export validated environment variables | ||
| const config = { | ||
| TURBOT_GRAPHQL_ENDPOINT: process.env.TURBOT_GRAPHQL_ENDPOINT!, | ||
| TURBOT_ACCESS_KEY_ID: process.env.TURBOT_ACCESS_KEY_ID!, | ||
| TURBOT_SECRET_ACCESS_KEY: process.env.TURBOT_SECRET_ACCESS_KEY!, | ||
| let config: any = { |
|
Please update the README with these new credential model. You should definitely include examples. I think the CLI profile method you've added should be the preferred / first example. |
Environment Configuration Changes Summary
1. Support for Two Credential Methods
TURBOT_GRAPHQL_ENDPOINTTURBOT_ACCESS_KEY_IDTURBOT_SECRET_ACCESS_KEYTURBOT_CLI_PROFILE(required)TURBOT_CLI_CREDENTIALS_PATH(optional, defaults to$HOME/.config/turbot/credentials.yml)2. Validation Logic
3. YAML Credentials Parsing
workspace→TURBOT_GRAPHQL_ENDPOINT(appends/api/latest/graphql)accessKey→TURBOT_ACCESS_KEY_IDsecretKey→TURBOT_SECRET_ACCESS_KEY4. Robustness and Error Handling
5. TypeScript Non-Null Assertion
!operator to assure TypeScript that environment variables are set after validation.Result: