feat(core): support organization API keys - #2021
Open
eoinest wants to merge 8 commits into
Open
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
size-limit report 📦
|
Contributor
Author
Contributor
Author
Contributor
Author
Contributor
Author
Comment on lines
+149
to
153
| this.apiKey ||= | ||
| process.env?.GT_API_KEY || | ||
| process.env?.GT_DEV_API_KEY || | ||
| process.env?.GT_ORG_API_KEY; | ||
| this.projectId ||= process.env?.GT_PROJECT_ID; |
Contributor
There was a problem hiding this comment.
GT_ORG_API_KEY silently ignored when GT_DEV_API_KEY is also set
GT_ORG_API_KEY is placed last in the || chain, so it is never evaluated when GT_DEV_API_KEY is present in the environment. A developer who moves to org-scoped authentication by adding GT_ORG_API_KEY to their environment but still has GT_DEV_API_KEY in their shell profile (common in dev setups) will have the org key silently dropped — API calls will authenticate with the old dev key, which may lack the org-level permissions needed, causing opaque auth failures.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/core/src/runtime.ts
Line: 149-153
Comment:
**`GT_ORG_API_KEY` silently ignored when `GT_DEV_API_KEY` is also set**
`GT_ORG_API_KEY` is placed last in the `||` chain, so it is never evaluated when `GT_DEV_API_KEY` is present in the environment. A developer who moves to org-scoped authentication by adding `GT_ORG_API_KEY` to their environment but still has `GT_DEV_API_KEY` in their shell profile (common in dev setups) will have the org key silently dropped — API calls will authenticate with the old dev key, which may lack the org-level permissions needed, causing opaque auth failures.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Contributor
Author
Contributor
Author
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
apiKey,devApiKey, andorgApiKeyas configuration aliases normalized into one stored API key.GT_ORG_API_KEYas an environment fallback while continuing to require a project ID for project operations.Testing
pnpm --filter generaltranslation test— passed (31 files, 504 tests)pnpm --filter generaltranslation typecheck— passedpnpm check:library-defaults— passed (4 tests)pnpm build— passed (20 packages)pnpm exec oxfmt --check packages/core/src/runtime.ts packages/core/src/__tests__/index.test.ts .changeset/tidy-oranges-agree.md— passedNotes
generaltranslation.Greptile Summary
This PR consolidates three API key aliases (
apiKey,devApiKey,orgApiKey) and the newGT_ORG_API_KEYenvironment variable into a singlethis.apiKeyfield, establishing a clear left-to-right priority (apiKey > devApiKey > orgApiKey, and env fallbacks in the same order). Explicit constructor/setConfigvalues now always override environment-loaded keys, fixing a previously-noted silent precedence bug.orgApiKeyalias: accepted in bothGTConstructorParamsandsetConfig, normalized intothis.apiKeywith the lowest priority among the three aliases;GT_ORG_API_KEYis added as an env fallback.devApiKeyfield removed: the publicdevApiKeyclass property onGTRuntimeis removed; both dev and org keys are now stored underthis.apiKey. This is a breaking API change for any consumer that readsinstance.devApiKeydirectly.patch, but the removal of the public field warrants at least aminorbump.Confidence Score: 4/5
Safe to merge after confirming the semver classification — the logic changes are correct and well-tested, but the changeset bump needs to be revisited.
The key-resolution logic is sound and the explicit-over-env priority is now correctly enforced. The only real concern is that removing the public
devApiKeyfield fromGTRuntimeis a breaking API surface change for TypeScript consumers who access that property directly, yet the changeset marks this as a patch release.Files Needing Attention:
packages/core/src/runtime.ts(removal ofdevApiKeypublic field) and.changeset/tidy-oranges-agree.md(version classification).Important Files Changed
this.apiKeyfield withpatch; removing the publicdevApiKeyfield makes this at least aminorbump.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Constructor called with GTConstructorParams] --> B{process defined?} B -- Yes --> C["this.apiKey ||= GT_API_KEY || GT_DEV_API_KEY || GT_ORG_API_KEY"] B -- No --> D[skip env read] C --> E[setConfig called with params] D --> E E --> F["this.apiKey = apiKey || devApiKey || orgApiKey || this.apiKey"] F --> G{Explicit key passed?} G -- Yes --> H[Explicit key wins - env key overwritten] G -- No --> I[Env key preserved] H --> J[_getTranslationConfig returns this.apiKey] I --> J J --> K{apiKey && projectId?} K -- No --> L[_validateAuth throws error] K -- Yes --> M[Translation request sent]Reviews (8): Last reviewed commit: "refactor(core): expose one API key field" | Re-trigger Greptile