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 refines the OAuth authentication process, particularly for scenarios where a web browser cannot be automatically launched. It introduces dedicated utility functions to provide clearer instructions to users and to intelligently determine when a manual callback URL prompt is necessary. This ensures a more robust and user-friendly login experience across different authentication providers by standardizing prompt behavior and guidance. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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. 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 refactors the OAuth manual prompt logic for command-line logins. The changes centralize the logic for when to display a manual prompt for pasting the OAuth callback URL, correctly tying it to the --no-browser flag. This fixes a bug where the prompt could be triggered incorrectly. The changes are consistent across multiple authentication providers and include new helper functions with corresponding tests. My feedback includes suggestions to improve the test structure to follow common Go practices for table-driven tests.
| for _, tt := range tests { | ||
| got := selectOAuthCallbackPrompt(tt.options, promptFn) | ||
| if (got == nil) != tt.wantNil { | ||
| t.Fatalf("%s: got nil=%v, want nil=%v", tt.name, got == nil, tt.wantNil) | ||
| } | ||
| } |
There was a problem hiding this comment.
For table-driven tests, it's a common Go practice to use t.Run to create sub-tests for each case. This allows all test cases to run even if one fails, and provides clearer output. Using t.Fatalf will stop the execution on the first failed test case.
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := selectOAuthCallbackPrompt(tt.options, promptFn)
if (got == nil) != tt.wantNil {
t.Errorf("got nil=%v, want nil=%v", got == nil, tt.wantNil)
}
})
}| for _, tt := range tests { | ||
| if got := ShouldPromptForOAuthCallback(tt.noBrowser, tt.promptFn); got != tt.want { | ||
| t.Fatalf("%s: got %v, want %v", tt.name, got, tt.want) | ||
| } | ||
| } |
There was a problem hiding this comment.
For table-driven tests, it's a common Go practice to use t.Run to create sub-tests for each case. This allows all test cases to run even if one fails, and provides clearer output. Using t.Fatalf will stop the execution on the first failed test case.
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := ShouldPromptForOAuthCallback(tt.noBrowser, tt.promptFn); got != tt.want {
t.Errorf("got %v, want %v", got, tt.want)
}
})
}![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: e41e23852b
ℹ️ 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".
| func ShouldPromptForOAuthCallback(noBrowser bool, promptFn func(string) (string, error)) bool { | ||
| return noBrowser && promptFn != nil |
There was a problem hiding this comment.
ShouldPromptForOAuthCallback now gates the manual callback prompt on noBrowser, but the auth flows in sdk/auth/{codex,claude,iflow,antigravity}.go and internal/auth/gemini/gemini_auth.go also fall back to a printed URL when browser.IsAvailable() is false or browser.OpenURL fails while leaving opts.NoBrowser as false. In those cases this change disables the only Paste the ... callback URL path after 15s, so headless/remote users who can authorize on another machine but cannot deliver the localhost redirect directly will now hang until timeout unless they rerun with --no-browser.
Useful? React with 👍 / 👎.
1 participant
No description provided.