Conversation
Chrome sync storage has an 8KB per-item limit which we were hitting with larger repo lists. This moves watchedRepos to local storage (unlimited quota) and adds a one-time migration for existing installs. Also added import validation to enforce the 50 repo limit (unless unlimited mode is enabled) and better error handling around the import flow.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the user experience and security by transitioning the GitHub authentication method from Personal Access Tokens to the GitHub OAuth device flow. This change simplifies the initial setup and ongoing management of GitHub connections within the extension. Additionally, the storage mechanism for watched repositories has been optimized to prevent quota limitations, ensuring smoother operation for users with extensive monitoring needs. The updates also include comprehensive adjustments to the user interface, error messaging, and documentation to reflect these core improvements. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a significant and well-executed refactoring that replaces the personal access token (PAT) authentication with a more secure and user-friendly GitHub OAuth device flow. The changes are extensive, touching documentation, background scripts, UI components, and storage logic. The introduction of a dedicated auth.js module for the device flow is a great architectural choice. Additionally, migrating the watchedRepos from sync to local storage is a thoughtful improvement for handling larger repository lists. The code is generally of high quality. I've included a few suggestions to improve maintainability by removing some redundant UI visibility logic.
| importSection.classList.toggle('hidden', !isConnected); | ||
| importSection.style.display = isConnected ? 'block' : 'none'; |
There was a problem hiding this comment.
The code uses both classList.toggle('hidden', ...) and style.display = ... to control the visibility of the importSection. The .hidden class in options.css already sets display: none;, making the direct manipulation of style.display redundant. This redundancy can make styling harder to debug and maintain. If the implementation of the .hidden class changes in the future (e.g., to use visibility), this code might break. It's better to rely solely on adding/removing the class for managing visibility.
| importSection.classList.toggle('hidden', !isConnected); | |
| importSection.style.display = isConnected ? 'block' : 'none'; | |
| importSection.classList.toggle('hidden', !isConnected); |
| deviceCodeSection.classList.toggle('hidden', !userCode); | ||
| deviceCodeSection.style.display = userCode ? 'block' : 'none'; |
There was a problem hiding this comment.
Similar to the handling of importSection, the visibility of deviceCodeSection is controlled using both classList.toggle and a direct style.display assignment. This is redundant and can lead to maintenance issues. Relying on just the CSS class is a cleaner and more maintainable approach.
| deviceCodeSection.classList.toggle('hidden', !userCode); | |
| deviceCodeSection.style.display = userCode ? 'block' : 'none'; | |
| deviceCodeSection.classList.toggle('hidden', !userCode); |
| importSection.classList.toggle('hidden', !hasStoredToken); | ||
| importSection.style.display = hasStoredToken ? 'block' : 'none'; |
There was a problem hiding this comment.
The syncTokenUiWithStoredCredential function uses both classList.toggle and a direct style.display assignment to manage the visibility of the import section. This is redundant since the .hidden class already handles the display property. To improve maintainability and rely on a single source of truth for styling (the CSS class), it's better to only use class manipulation.
| importSection.classList.toggle('hidden', !hasStoredToken); | |
| importSection.style.display = hasStoredToken ? 'block' : 'none'; | |
| importSection.classList.toggle('hidden', !hasStoredToken); |
There was a problem hiding this comment.
Code Review
This pull request is a significant and well-executed improvement, replacing the manual personal access token flow with a much cleaner GitHub OAuth device flow. The related changes to storage, UI, and documentation are thorough and consistent. My review includes a couple of suggestions to refactor small areas of duplicated code and simplify data flow, which will enhance the long-term maintainability of the new options page logic.
| function syncTokenUiWithStoredCredential(hasStoredToken) { | ||
| const clearTokenBtn = document.getElementById('clearTokenBtn'); | ||
| const connectGitHubBtn = document.getElementById('connectGitHubBtn'); | ||
| const repoInput = document.getElementById('repoInput'); | ||
| const addRepoBtn = document.getElementById('addRepoBtn'); | ||
| const repoHelpText = document.getElementById('repoHelpText'); | ||
| const importSection = document.getElementById('importReposSection'); | ||
|
|
||
| connectGitHubBtn.textContent = hasStoredToken ? 'Reconnect GitHub' : 'Connect GitHub'; | ||
| clearTokenBtn.style.display = hasStoredToken ? 'block' : 'none'; | ||
| repoInput.disabled = !hasStoredToken; | ||
| repoInput.placeholder = hasStoredToken | ||
| ? 'e.g., react, facebook/react, or GitHub URL' | ||
| : 'Enter a valid GitHub token to add repositories'; | ||
| : 'Connect GitHub to add repositories'; | ||
| addRepoBtn.disabled = !hasStoredToken; | ||
| repoHelpText.textContent = hasStoredToken | ||
| ? 'Add repositories to monitor (npm package, owner/repo, or GitHub URL)' | ||
| : 'Add a valid GitHub token above to start adding repositories'; | ||
| : 'Connect GitHub above to start adding repositories'; | ||
| importSection.classList.toggle('hidden', !hasStoredToken); | ||
| importSection.style.display = hasStoredToken ? 'block' : 'none'; | ||
| } |
There was a problem hiding this comment.
This function's logic is duplicated in applyStoredConnection within options/controllers/token-controller.js. To improve maintainability and reduce code duplication, you can replace the call to syncTokenUiWithStoredCredential(false) in loadSettings with applyStoredConnection(null) and then remove this function entirely.
|
|
||
| await chrome.storage.sync.set({ watchedRepos }); | ||
| await setWatchedRepos(nextWatchedRepos); | ||
| watchedRepos.splice(0, watchedRepos.length, ...nextWatchedRepos); |
There was a problem hiding this comment.
This line mutates the watchedRepos array in-place. However, the calling code in options.js provides a callback that immediately re-fetches the repositories from storage, overwriting this change. This makes the in-place mutation redundant. Removing this line would simplify the function by making the data flow more explicit and avoiding an unnecessary side-effect.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 040192e020
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| function isConfiguredClientId(clientId) { | ||
| return Boolean(clientId) && clientId !== OAUTH_CONFIG.CLIENT_ID; |
There was a problem hiding this comment.
getGitHubOAuthClientId() falls back to OAUTH_CONFIG.CLIENT_ID when no storage override exists, but isConfiguredClientId() rejects any value equal to OAUTH_CONFIG.CLIENT_ID. As written, requireGitHubOAuthClientId() will always throw client_id_missing unless a different ID is manually written into chrome.storage, so the new GitHub device sign-in flow cannot start in a normal install.
Useful? React with 👍 / 👎.
1 participant
Description
Replace the manual personal access token setup with a GitHub OAuth device flow across onboarding and settings, then update storage so auth and larger repo lists are handled more cleanly.
Changes
Testing
npm run lintnpm run typechecknpm test -- --runInBandnpm run buildnpm run validateScreenshots
Checklist