fix(core): preserve OAuth refresh tokens during rotation and retrieval#26924
Conversation
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 improves the reliability of OAuth credential management by ensuring that refresh tokens are not discarded during rotation or retrieval. By allowing the system to retain and recognize expired tokens that still possess a valid refresh token, the application can now perform silent background refreshes, significantly improving the user experience by reducing unnecessary re-authentication prompts. Highlights
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 the 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 counterproductive. 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. 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 addresses issue #25460 by ensuring that OAuth refresh tokens are preserved during credential updates and that expired credentials are not discarded if a refresh token is available. Specifically, it modifies OAuthCredentialStorage and MCPOAuthTokenStorage to merge existing refresh tokens when saving new credentials and updates KeychainTokenStorage to return expired tokens if a refresh token is present. A new reproduction test and updates to existing tests have been added to verify the fix. I have no feedback to provide as no review comments were submitted.
|
Size Change: +454 B (0%) Total Size: 34.1 MB
ℹ️ View Unchanged
|
gemini-cli
Bot
added
priority/p2
2 participants
Summary
This PR addresses two critical defects in OAuth credential handling where valid refresh tokens were being discarded, forcing users into full re-authentication flows instead of silently refreshing.
OAuthCredentialStorage.saveCredentialsandMCPOAuthTokenStorage.saveTokennow correctly merge the existingrefreshTokenfrom storage if the incoming payload (like a rotation response) omits it.KeychainTokenStorage.getCredentialsandgetAllCredentialsno longer drop expired credentials if they contain a validrefreshToken.Details
Historically,
KeychainTokenStorageimplemented a strict policy of returningnullfor any expired token. This was too blunt, as it hid "refreshable" tokens (those with arefresh_token) from the domain adapters (MCPOAuthProviderandOAuth2AuthProvider) that rely on receiving that expired state to trigger a background refresh.Additionally, the save methods were blindly overwriting storage with the new token payload, which often lacks a
refresh_tokenduring a standard Google OAuth rotation, causing the persistent token to be lost.This fix delegates the responsibility of merging state to the domain adapters and relaxes the storage layer to return expired but refreshable tokens.
Related Issues
Closes #25460
How to Validate
npm test -w @google/gemini-cli-coreto verify the new reproduction unit tests pass:Issue #25460 Reproduction > OAuthCredentialStorage.saveCredentials > overwrites refresh token when new payload lacks oneIssue #25460 Reproduction > KeychainTokenStorage.getCredentials > discards expired credentials even if refresh_token is presentgemini auth login.gemini ask "hello". The CLI should silently refresh the token rather than opening the browser.Pre-Merge Checklist