fix(ui): handle headless execution in credits and upgrade dialogs#21850
Conversation
Add shouldLaunchBrowser() checks to openG1Url(), /upgrade command, and handleUpgrade() fallback handler. When browser cannot be launched (CI, SSH, headless), URLs are printed to the user instead of silently failing.
gsquared94
force-pushed
the
fix/headless-credits-dialog-browser
branch
from
1935531 to
890cadc
Compare
gemini-cli
bot
added
the
status/need-issue
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 improves the user experience in headless environments (such as CI, SSH, or other non-GUI setups) by preventing silent failures when browser-dependent actions are triggered. Instead of attempting to open a browser, the system now gracefully provides the relevant URLs directly to the user via chat messages or logs, ensuring that critical information like upgrade pages or credit management links remains accessible. 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 effectively addresses headless execution for credit and upgrade dialogs by printing URLs instead of attempting to open a browser. The changes are consistently applied across the relevant files and are well-supported by new unit tests. The modification to openG1Url to conditionally return the URL is a clean solution. I've identified one area for improvement regarding test coverage.
| })); | ||
| vi.mock('../utils/secure-browser-launcher.js', () => ({ | ||
| openBrowserSecurely: vi.fn(), | ||
| shouldLaunchBrowser: vi.fn().mockReturnValue(true), |
There was a problem hiding this comment.
While shouldLaunchBrowser is now mocked, the new headless logic in handleUpgrade within packages/core/src/fallback/handler.ts is not covered by a test case. This leaves the new functionality in handleUpgrade untested.
Please add a test case to verify this behavior. When constructing the test, prefer using hardcoded literal values instead of importing constants like UPGRADE_URL_PAGE to ensure tests are self-contained and less brittle. For example:
it('should log URL and not open browser for upgrade intent when headless', async () => {
const { shouldLaunchBrowser } = await import(
'../utils/secure-browser-launcher.js'
);
vi.mocked(shouldLaunchBrowser).mockReturnValue(false);
policyHandler.mockResolvedValue('upgrade');
const result = await handleFallback(
policyConfig,
MOCK_PRO_MODEL,
AUTH_OAUTH,
);
expect(result).toBe(false);
expect(openBrowserSecurely).not.toHaveBeenCalled();
expect(debugLogger.log).toHaveBeenCalledWith(
`Cannot open browser in this environment. Please visit: https://example.com/upgrade-page-url`, // Replace with the actual literal URL
);
});References
- In tests, prefer using hardcoded literal values instead of importing constants to ensure tests are self-contained and less brittle.
2 participants
Summary
Fix headless execution handling in AI credits integration dialogs and the
/upgradecommand. WhenshouldLaunchBrowser()returnsfalse(CI, SSH, headless environments), the credits "Manage" and "Get Credits" options, as well as the/upgradecommand and corehandleUpgrade()fallback, now print the URL to the user instead of silently failing.Details
Three places called
openBrowserSecurely()without checkingshouldLaunchBrowser()first:creditsFlowHandler.tsopenG1Url()— Used by "Manage" (overage menu) and "Get Credits" (empty wallet menu). Changed return type toPromise<string | undefined>so callers can surface the URL as an info message in chat history.upgradeCommand.ts— The/upgradeslash command now returns an info message with the URL when browser can't launch.handler.tshandleUpgrade()— Core-level handler logs the URL viadebugLogger.log()since it has no UI access.This follows the patterns already established in
ValidationDialog.tsxandBannedAccountDialog.tsx.Related Issues
N/A
How to Validate
CI=1in your environment and trigger a quota error to see the overage/empty wallet dialogs print URLs instead of trying to open a browser.CI=1, run/upgradeand verify it returns the URL as an info message.Pre-Merge Checklist