-
-
Notifications
You must be signed in to change notification settings - Fork 551
feat: add direct refresh token input for adding accounts #237
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
feat: add direct refresh token input for adding accounts #237
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds direct refresh-token authentication across the codebase. Introduces validateRefreshToken(refreshToken, projectId) in src/antigravity/oauth.ts to exchange a Google refresh token for an access token, fetch user info, resolve projectId (optionally via fetchProjectID), persist the refresh token with the projectId, and return a structured token exchange result with error handling. Extends plugin flows in src/plugin.ts to offer and persist refresh-token accounts (including a new public OAuth method "Direct Refresh Token Input") and adds CLI prompts in src/plugin/cli.ts: promptAuthMethod() to choose auth type and promptRefreshToken() to collect a token. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryAdded direct refresh token input as an authentication method, allowing users to add accounts by pasting existing Google refresh tokens. This is particularly useful for headless/SSH environments where browser OAuth is difficult. Key Changes:
Implementation Notes:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant CLI as CLI/TUI
participant Plugin as plugin.ts
participant OAuth as oauth.ts
participant API as Google API
participant Storage as Storage
alt CLI Flow
User->>CLI: Run auth login
CLI->>Plugin: Prompt auth method
User->>Plugin: Choose direct input
Plugin->>CLI: Request input
User->>Plugin: Provide input
else TUI Flow
User->>Plugin: Choose direct input
Plugin-->>User: Show instructions
User->>Plugin: Provide input
end
Plugin->>OAuth: Process input
OAuth->>API: Request authentication
API-->>OAuth: Return result
OAuth->>API: Fetch user info
API-->>OAuth: Return user info
OAuth->>API: Fetch project info
API-->>OAuth: Return project info
OAuth-->>Plugin: Return complete result
Plugin->>Storage: Load accounts
Storage-->>Plugin: Return accounts
Plugin->>Plugin: Check duplicates
alt Duplicate exists
Plugin-->>User: Show error
else New account
Plugin->>Storage: Save account
Plugin-->>User: Show success
end
|
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.
3 files reviewed, 2 comments
src/plugin.ts
Outdated
| // Check if account already exists | ||
| const existingStorage = await loadAccounts(); | ||
| const existingAccount = existingStorage?.accounts.find( | ||
| acc => acc.email === result.email || acc.refreshToken === result.refresh |
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.
logic: Comparison with result.refresh is incorrect - should compare with the raw refresh token
The result.refresh includes the project ID in format "token|projectId" (see line 267 in oauth.ts), but acc.refreshToken stores just the token part without the pipe separator. This comparison will always fail.
| acc => acc.email === result.email || acc.refreshToken === result.refresh | |
| acc => acc.email === result.email || acc.refreshToken === refreshToken |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/plugin.ts
Line: 2093:2093
Comment:
**logic:** Comparison with `result.refresh` is incorrect - should compare with the raw refresh token
The `result.refresh` includes the project ID in format `"token|projectId"` (see line 267 in oauth.ts), but `acc.refreshToken` stores just the token part without the pipe separator. This comparison will always fail.
```suggestion
acc => acc.email === result.email || acc.refreshToken === refreshToken
```
How can I resolve this? If you propose a fix, please make it concise.| if (authMethod === "refresh_token") { | ||
| // Direct refresh token flow | ||
| const refreshToken = await promptRefreshToken(); | ||
| if (!refreshToken) { | ||
| console.log("No refresh token provided, skipping..."); | ||
| continue; | ||
| } | ||
|
|
||
| const projectId = await promptProjectId(); | ||
| console.log("\nValidating refresh token..."); | ||
|
|
||
| const result = await validateRefreshToken(refreshToken, projectId); | ||
|
|
||
| if (result.type === "failed") { | ||
| console.log(`\n❌ Refresh token validation failed: ${result.error}`); | ||
| if (accounts.length === 0) { | ||
| return { | ||
| url: "", | ||
| instructions: `Authentication failed: ${result.error}`, | ||
| method: "auto", | ||
| callback: async () => result, | ||
| }; | ||
| } | ||
| console.log("Skipping this account...\n"); | ||
| continue; | ||
| } | ||
|
|
||
| console.log(`\n✓ Token validated successfully${result.email ? ` (${result.email})` : ""}`); | ||
| accounts.push(result); | ||
|
|
||
| // Show toast for successful account authentication | ||
| try { | ||
| await client.tui.showToast({ | ||
| body: { | ||
| message: `Account ${accounts.length} authenticated${result.email ? ` (${result.email})` : ""}`, | ||
| variant: "success", | ||
| }, | ||
| }); | ||
| } catch { | ||
| // TUI may not be available in CLI mode | ||
| } | ||
|
|
||
| try { | ||
| const isFirstAccount = accounts.length === 1; | ||
| await persistAccountPool([result], isFirstAccount && startFresh); | ||
| } catch { | ||
| // ignore | ||
| } | ||
|
|
||
| if (accounts.length >= MAX_OAUTH_ACCOUNTS) { | ||
| break; | ||
| } | ||
|
|
||
| let currentAccountCount = accounts.length; | ||
| try { | ||
| const currentStorage = await loadAccounts(); | ||
| if (currentStorage) { | ||
| currentAccountCount = currentStorage.accounts.length; | ||
| } | ||
| } catch { | ||
| // Fall back to accounts.length if we can't read storage | ||
| } | ||
|
|
||
| const addAnother = await promptAddAnotherAccount(currentAccountCount); | ||
| if (!addAnother) { | ||
| break; | ||
| } | ||
| continue; | ||
| } |
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.
style: Missing duplicate account check in CLI flow
The TUI flow (lines 2090-2108) checks for duplicate accounts before adding, but the CLI flow doesn't perform this check. This means users could accidentally add the same account multiple times in CLI mode, even though persistAccountPool() will deduplicate by email/token later. Consider adding the same duplicate check here for consistency and better user experience.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/plugin.ts
Line: 1678:1746
Comment:
**style:** Missing duplicate account check in CLI flow
The TUI flow (lines 2090-2108) checks for duplicate accounts before adding, but the CLI flow doesn't perform this check. This means users could accidentally add the same account multiple times in CLI mode, even though `persistAccountPool()` will deduplicate by email/token later. Consider adding the same duplicate check here for consistency and better user experience.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.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.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/plugin.ts`:
- Around line 2073-2074: The instructions string assigned to the "instructions"
field in src/plugin.ts currently hardcodes a Windows-only path
(%APPDATA%\\opencode\\antigravity-accounts.json); change this to a
cross-platform approach by describing or programmatically resolving the accounts
file location instead of a Windows-only literal. Update the "instructions" value
(near the URL
"https://accounts.google.com/o/oauth2/v2/auth?note=paste_your_refresh_token_below")
to either (a) explain where to find the file on each platform (Windows:
%APPDATA%/opencode/antigravity-accounts.json, macOS: ~/Library/Application
Support/opencode/antigravity-accounts.json, Linux:
$XDG_CONFIG_HOME/opencode/antigravity-accounts.json or
~/.config/opencode/antigravity-accounts.json) or (b) reference the
runtime-resolved path (e.g., using
os.homedir()/process.env.XDG_CONFIG_HOME/process.env.APPDATA) and show a
placeholder like "<resolved path>" so users on any OS know where to look.
- Around line 2091-2094: The duplicate-detection is comparing acc.refreshToken
(raw token) to result.refresh (composite
`${refreshToken}|${effectiveProjectId}`) so it will never match; fix by
comparing like-for-like: obtain the raw refresh token (either capture it before
calling validateRefreshToken or derive it from result.refresh by splitting on
'|' and taking the first segment) and use that raw token in the existingAccount
check (the code around loadAccounts(), existingStorage, existingAccount and
validateRefreshToken should be updated so acc.refreshToken === rawRefreshToken
rather than acc.refreshToken === result.refresh).
In `@src/plugin/cli.ts`:
- Line 70: The file defines a primitive union type AuthMethod = "oauth" |
"refresh_token" which conflicts with the existing AuthMethod interface in
src/plugin/types.ts; rename the union to a non-conflicting name (e.g.,
AuthMethodType or AuthStrategy) and update all references accordingly: change
the type declaration in src/plugin/cli.ts, update the return type of
promptAuthMethod(), and adjust any usages in src/plugin.ts to import/use the new
name to avoid ambiguity with the AuthMethod interface.
🧹 Nitpick comments (3)
src/antigravity/oauth.ts (1)
198-283: LGTM with minor concern: Missing fetch timeout.The implementation is solid with proper error handling and matches the patterns used in
exchangeAntigravity. However, the fetch calls at lines 210 and 247 don't usefetchWithTimeout(defined at line 117) unlike other network calls in this file.Consider using
fetchWithTimeoutfor consistency:♻️ Suggested improvement for timeout consistency
- const tokenResponse = await fetch("https://oauth2.googleapis.com/token", { + const tokenResponse = await fetchWithTimeout("https://oauth2.googleapis.com/token", { method: "POST", headers: { "Content-Type": "application/x-www-form-urlencoded", }, body: new URLSearchParams({ client_id: ANTIGRAVITY_CLIENT_ID, client_secret: ANTIGRAVITY_CLIENT_SECRET, refresh_token: refreshToken, grant_type: "refresh_token", }), });And similarly for the user info fetch at line 247.
src/plugin.ts (2)
2082-2085: Overly restrictive token format validation.The check
!refreshToken.startsWith("1//")may reject valid Google refresh tokens. While1//is common, Google's token format can vary. Consider either removing this check (relying on server validation) or making it less restrictive.♻️ Suggested alternatives
Option 1: Remove format check (recommended) - let server validate:
- // Check if it looks like a refresh token - if (!refreshToken.startsWith("1//")) { - return { type: "failed", error: "Invalid refresh token format. Token should start with '1//'" }; - }Option 2: Warn instead of reject:
// Check if it looks like a refresh token if (!refreshToken.startsWith("1//")) { - return { type: "failed", error: "Invalid refresh token format. Token should start with '1//'" }; + // Warn but continue - let server validate the actual token + try { + await client.tui.showToast({ + body: { message: "Token format looks unusual, attempting validation...", variant: "warning" }, + }); + } catch {} }
2068-2135: Consider extracting duplicate detection logic.The duplicate detection at lines 2091-2108 could be reused from the CLI flow or extracted into a helper function to maintain consistency and reduce duplication.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (3)
src/antigravity/oauth.tssrc/plugin.tssrc/plugin/cli.ts
🧰 Additional context used
🧬 Code graph analysis (3)
src/plugin/cli.ts (1)
src/plugin/types.ts (1)
AuthMethod(77-83)
src/antigravity/oauth.ts (3)
index.ts (1)
AntigravityTokenExchangeResult(13-13)src/constants.ts (2)
ANTIGRAVITY_CLIENT_ID(4-4)ANTIGRAVITY_CLIENT_SECRET(9-9)src/plugin/auth.ts (1)
calculateTokenExpiry(45-52)
src/plugin.ts (5)
src/plugin/cli.ts (4)
promptAuthMethod(76-100)promptRefreshToken(105-115)promptProjectId(7-15)promptAddAnotherAccount(20-29)src/plugin/refresh-queue.ts (1)
refreshToken(209-224)src/antigravity/oauth.ts (2)
validateRefreshToken(202-283)AntigravityTokenExchangeResult(50-52)src/plugin/storage.ts (1)
loadAccounts(433-518)index.ts (1)
AntigravityTokenExchangeResult(13-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Greptile Review
🔇 Additional comments (3)
src/plugin/cli.ts (2)
76-100: LGTM!The
promptAuthMethodfunction is well-implemented following the existing patterns in this file (readline interface creation, proper cleanup in finally block, input validation loop).
102-115: LGTM!The
promptRefreshTokenfunction follows the established patterns with proper resource cleanup.src/plugin.ts (1)
1673-1746: LGTM!The CLI flow for direct refresh token input is well-implemented with proper validation, error handling, account persistence, and toast notifications. The flow correctly handles both success and failure cases, and integrates well with the existing multi-account authentication loop.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
d7675c5 to
a189f32
Compare
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.
3 files reviewed, 1 comment
| const refreshToken = await promptRefreshToken(); | ||
| if (!refreshToken) { | ||
| console.log("No refresh token provided, skipping..."); | ||
| continue; | ||
| } | ||
|
|
||
| const projectId = await promptProjectId(); | ||
| console.log("\nValidating refresh token..."); | ||
|
|
||
| const result = await validateRefreshToken(refreshToken, projectId); |
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.
style: Missing token format validation in CLI flow
The TUI flow validates that tokens start with 1// (line 2106), but the CLI flow skips this check. Users could input invalid tokens and only discover the issue after the API call.
| const refreshToken = await promptRefreshToken(); | |
| if (!refreshToken) { | |
| console.log("No refresh token provided, skipping..."); | |
| continue; | |
| } | |
| const projectId = await promptProjectId(); | |
| console.log("\nValidating refresh token..."); | |
| const result = await validateRefreshToken(refreshToken, projectId); | |
| const refreshToken = await promptRefreshToken(); | |
| if (!refreshToken) { | |
| console.log("No refresh token provided, skipping..."); | |
| continue; | |
| } | |
| // Check if it looks like a refresh token | |
| if (!refreshToken.startsWith("1//")) { | |
| console.log("\n❌ Invalid refresh token format. Token should start with '1//'"); | |
| if (accounts.length === 0) { | |
| return { | |
| url: "", | |
| instructions: "Invalid refresh token format. Token should start with '1//'", | |
| method: "auto", | |
| callback: async () => ({ type: "failed", error: "Invalid refresh token format" }), | |
| }; | |
| } | |
| console.log("Skipping this account...\n"); | |
| continue; | |
| } | |
| const projectId = await promptProjectId(); | |
| console.log("\nValidating refresh token..."); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/plugin.ts
Line: 1680:1689
Comment:
**style:** Missing token format validation in CLI flow
The TUI flow validates that tokens start with `1//` (line 2106), but the CLI flow skips this check. Users could input invalid tokens and only discover the issue after the API call.
```suggestion
const refreshToken = await promptRefreshToken();
if (!refreshToken) {
console.log("No refresh token provided, skipping...");
continue;
}
// Check if it looks like a refresh token
if (!refreshToken.startsWith("1//")) {
console.log("\n❌ Invalid refresh token format. Token should start with '1//'");
if (accounts.length === 0) {
return {
url: "",
instructions: "Invalid refresh token format. Token should start with '1//'",
method: "auto",
callback: async () => ({ type: "failed", error: "Invalid refresh token format" }),
};
}
console.log("Skipping this account...\n");
continue;
}
const projectId = await promptProjectId();
console.log("\nValidating refresh token...");
```
How can I resolve this? If you propose a fix, please make it concise.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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/plugin.ts`:
- Around line 1705-1721: The account-existence check currently always loads
existingStorage and skips accounts even in "fresh" mode; change the logic so
loadAccounts() and the existingAccount detection (the
parseRefreshParts/acc.refreshToken/email comparisons) are only performed when
the fresh-start flag is false—i.e., wrap the call to loadAccounts() and the
subsequent existingAccount check in a guard like if (!freshStart) (or the
appropriate boolean flag in scope such as isFresh/isFreshMode) so when
fresh-start is selected the code does not skip adding accounts.
♻️ Duplicate comments (1)
src/plugin.ts (1)
2090-2098: Path hint is still Linux/Windows-only.macOS and XDG locations are not covered, so the hint will be misleading for those users.
🔧 Cross-platform hint
- const pathHint = process.platform === "win32" - ? "%APPDATA%\\opencode\\antigravity-accounts.json" - : "~/.config/opencode/antigravity-accounts.json"; + const pathHint = process.platform === "win32" + ? "%APPDATA%\\opencode\\antigravity-accounts.json" + : process.platform === "darwin" + ? "~/Library/Application Support/opencode/antigravity-accounts.json" + : "$XDG_CONFIG_HOME/opencode/antigravity-accounts.json (or ~/.config/opencode/antigravity-accounts.json)";
🧹 Nitpick comments (1)
src/plugin/cli.ts (1)
84-93: Normalize auth-method input to avoid case-sensitive rejects.Line 86 uses
trim()only, so inputs like “OAuth” or “TOKEN” won’t match. Lower-casing keeps UX consistent with other prompts.♻️ Suggested tweak
- const normalized = answer.trim(); + const normalized = answer.trim().toLowerCase();
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
src/antigravity/oauth.tssrc/plugin.tssrc/plugin/cli.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/antigravity/oauth.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Greptile Review
🔇 Additional comments (1)
src/plugin/cli.ts (1)
105-111: Mask refresh-token input to avoid leaking secrets in terminal.Line 110 echoes the token back to the screen. Refresh tokens are sensitive and should be hidden while typing. However, the suggested fix using
hideEchoBackwon't work—Node's built-inreadline/promisesdoesn't support that option. Therl.question()method only accepts asignaloption for AbortSignal. To hide input, you'll need either a third-party package likereadline-syncor a custom TTY handling approach.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| // Check if account already exists by email or by raw refresh token | ||
| const existingStorage = await loadAccounts(); | ||
| const inputTokenParts = parseRefreshParts(result.refresh); | ||
| const existingAccount = existingStorage?.accounts.find(acc => { | ||
| if (acc.email && result.email && acc.email === result.email) { | ||
| return true; | ||
| } | ||
| // Compare raw refresh tokens (without projectId suffix) | ||
| const storedParts = parseRefreshParts(acc.refreshToken); | ||
| return storedParts.refreshToken === inputTokenParts.refreshToken; | ||
| }); | ||
|
|
||
| if (existingAccount) { | ||
| console.log(`\n⚠️ Account already exists: ${result.email || "unknown"}`); | ||
| console.log("Skipping this account...\n"); | ||
| continue; | ||
| } |
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.
Fresh-start mode still blocks re-adding existing accounts.
Line 1706 loads existing storage even when the user chose “fresh”. If the first account matches an existing email/token, it will be skipped, preventing a true fresh start.
🐛 Suggested fix
- const existingStorage = await loadAccounts();
+ const existingStorage =
+ startFresh && accounts.length === 0 ? null : await loadAccounts();🤖 Prompt for AI Agents
In `@src/plugin.ts` around lines 1705 - 1721, The account-existence check
currently always loads existingStorage and skips accounts even in "fresh" mode;
change the logic so loadAccounts() and the existingAccount detection (the
parseRefreshParts/acc.refreshToken/email comparisons) are only performed when
the fresh-start flag is false—i.e., wrap the call to loadAccounts() and the
subsequent existingAccount check in a guard like if (!freshStart) (or the
appropriate boolean flag in scope such as isFresh/isFreshMode) so when
fresh-start is selected the code does not skip adding accounts.
This feature allows users to add accounts by directly pasting a Google refresh token instead of going through the OAuth browser flow. Changes: - Add promptAuthMethod() and promptRefreshToken() functions in cli.ts - Add validateRefreshToken() function in oauth.ts to validate tokens - Add "Direct Refresh Token Input" option in the login methods - Support both CLI and TUI flows for refresh token input - Check for duplicate accounts before adding (by email or token) - Use cross-platform path hints for token file location - Rename AuthMethod to LoginAuthMethod to avoid type conflicts This is useful for: - Headless/SSH environments where browser auth is difficult - Sharing tokens across machines - Quickly adding accounts from existing token storage Co-Authored-By: Claude <[email protected]>
a189f32 to
0925076
Compare
Greptile's behavior is changing!From now on, if a review finishes with no comments, we will not post an additional "statistics" comment to confirm that our review found nothing to comment on. However, you can confirm that we reviewed your changes in the status check section. This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR". |
|
I just changed merge into dev also check if this is conflict with #231 |
Signed-off-by: apple246680 <[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.
3 files reviewed, 2 comments
Summary
Use Cases
Changes
src/plugin/cli.ts: AddpromptAuthMethod()andpromptRefreshToken()functionssrc/antigravity/oauth.ts: AddvalidateRefreshToken()function to validate tokens and fetch user infosrc/plugin.ts: Integrate refresh token authentication option in both CLI and TUI flowsTesting
opencode auth login1//0eXXX...)