fix(cli): update F12 behavior to only open drawer if browser fails (#18829)

This commit is contained in:
Sandy Tao
2026-02-11 09:45:43 -08:00
committed by GitHub
parent eb9223b6a4
commit b19b026f30
3 changed files with 42 additions and 15 deletions

View File

@@ -1521,6 +1521,7 @@ Logging in with Google... Restarting Gemini CLI to continue.
); );
await toggleDevToolsPanel( await toggleDevToolsPanel(
config, config,
showErrorDetails,
() => setShowErrorDetails((prev) => !prev), () => setShowErrorDetails((prev) => !prev),
() => setShowErrorDetails(true), () => setShowErrorDetails(true),
); );
@@ -1660,6 +1661,7 @@ Logging in with Google... Restarting Gemini CLI to continue.
tabFocusTimeoutRef, tabFocusTimeoutRef,
showTransientMessage, showTransientMessage,
settings.merged.general.devtools, settings.merged.general.devtools,
showErrorDetails,
], ],
); );

View File

@@ -437,7 +437,19 @@ describe('devtoolsService', () => {
}); });
describe('toggleDevToolsPanel', () => { describe('toggleDevToolsPanel', () => {
it('calls toggle when browser opens successfully', async () => { it('calls toggle (to close) when already open', async () => {
const config = createMockConfig();
const toggle = vi.fn();
const setOpen = vi.fn();
const promise = toggleDevToolsPanel(config, true, toggle, setOpen);
await promise;
expect(toggle).toHaveBeenCalledTimes(1);
expect(setOpen).not.toHaveBeenCalled();
});
it('does NOT call toggle or setOpen when browser opens successfully', async () => {
const config = createMockConfig(); const config = createMockConfig();
const toggle = vi.fn(); const toggle = vi.fn();
const setOpen = vi.fn(); const setOpen = vi.fn();
@@ -447,18 +459,18 @@ describe('devtoolsService', () => {
mockDevToolsInstance.start.mockResolvedValue('http://127.0.0.1:25417'); mockDevToolsInstance.start.mockResolvedValue('http://127.0.0.1:25417');
mockDevToolsInstance.getPort.mockReturnValue(25417); mockDevToolsInstance.getPort.mockReturnValue(25417);
const promise = toggleDevToolsPanel(config, toggle, setOpen); const promise = toggleDevToolsPanel(config, false, toggle, setOpen);
await vi.waitFor(() => expect(MockWebSocket.instances.length).toBe(1)); await vi.waitFor(() => expect(MockWebSocket.instances.length).toBe(1));
MockWebSocket.instances[0].simulateError(); MockWebSocket.instances[0].simulateError();
await promise; await promise;
expect(toggle).toHaveBeenCalledTimes(1); expect(toggle).not.toHaveBeenCalled();
expect(setOpen).not.toHaveBeenCalled(); expect(setOpen).not.toHaveBeenCalled();
}); });
it('calls toggle when browser fails to open', async () => { it('calls setOpen when browser fails to open', async () => {
const config = createMockConfig(); const config = createMockConfig();
const toggle = vi.fn(); const toggle = vi.fn();
const setOpen = vi.fn(); const setOpen = vi.fn();
@@ -468,18 +480,18 @@ describe('devtoolsService', () => {
mockDevToolsInstance.start.mockResolvedValue('http://127.0.0.1:25417'); mockDevToolsInstance.start.mockResolvedValue('http://127.0.0.1:25417');
mockDevToolsInstance.getPort.mockReturnValue(25417); mockDevToolsInstance.getPort.mockReturnValue(25417);
const promise = toggleDevToolsPanel(config, toggle, setOpen); const promise = toggleDevToolsPanel(config, false, toggle, setOpen);
await vi.waitFor(() => expect(MockWebSocket.instances.length).toBe(1)); await vi.waitFor(() => expect(MockWebSocket.instances.length).toBe(1));
MockWebSocket.instances[0].simulateError(); MockWebSocket.instances[0].simulateError();
await promise; await promise;
expect(toggle).toHaveBeenCalledTimes(1); expect(toggle).not.toHaveBeenCalled();
expect(setOpen).not.toHaveBeenCalled(); expect(setOpen).toHaveBeenCalledTimes(1);
}); });
it('calls toggle when shouldLaunchBrowser returns false', async () => { it('calls setOpen when shouldLaunchBrowser returns false', async () => {
const config = createMockConfig(); const config = createMockConfig();
const toggle = vi.fn(); const toggle = vi.fn();
const setOpen = vi.fn(); const setOpen = vi.fn();
@@ -488,15 +500,15 @@ describe('devtoolsService', () => {
mockDevToolsInstance.start.mockResolvedValue('http://127.0.0.1:25417'); mockDevToolsInstance.start.mockResolvedValue('http://127.0.0.1:25417');
mockDevToolsInstance.getPort.mockReturnValue(25417); mockDevToolsInstance.getPort.mockReturnValue(25417);
const promise = toggleDevToolsPanel(config, toggle, setOpen); const promise = toggleDevToolsPanel(config, false, toggle, setOpen);
await vi.waitFor(() => expect(MockWebSocket.instances.length).toBe(1)); await vi.waitFor(() => expect(MockWebSocket.instances.length).toBe(1));
MockWebSocket.instances[0].simulateError(); MockWebSocket.instances[0].simulateError();
await promise; await promise;
expect(toggle).toHaveBeenCalledTimes(1); expect(toggle).not.toHaveBeenCalled();
expect(setOpen).not.toHaveBeenCalled(); expect(setOpen).toHaveBeenCalledTimes(1);
}); });
it('calls setOpen when DevTools server fails to start', async () => { it('calls setOpen when DevTools server fails to start', async () => {
@@ -506,7 +518,7 @@ describe('devtoolsService', () => {
mockDevToolsInstance.start.mockRejectedValue(new Error('fail')); mockDevToolsInstance.start.mockRejectedValue(new Error('fail'));
const promise = toggleDevToolsPanel(config, toggle, setOpen); const promise = toggleDevToolsPanel(config, false, toggle, setOpen);
await vi.waitFor(() => expect(MockWebSocket.instances.length).toBe(1)); await vi.waitFor(() => expect(MockWebSocket.instances.length).toBe(1));
MockWebSocket.instances[0].simulateError(); MockWebSocket.instances[0].simulateError();

View File

@@ -212,14 +212,24 @@ async function startDevToolsServerImpl(config: Config): Promise<string> {
/** /**
* Handles the F12 key toggle for the DevTools panel. * Handles the F12 key toggle for the DevTools panel.
* Starts the DevTools server, attempts to open the browser, * Starts the DevTools server, attempts to open the browser.
* and always calls the toggle callback regardless of the outcome. * If the panel is already open, it closes it.
* If the panel is closed:
* - Attempts to open the browser.
* - If browser opening is successful, the panel remains closed.
* - If browser opening fails or is not possible, the panel is opened.
*/ */
export async function toggleDevToolsPanel( export async function toggleDevToolsPanel(
config: Config, config: Config,
isOpen: boolean,
toggle: () => void, toggle: () => void,
setOpen: () => void, setOpen: () => void,
): Promise<void> { ): Promise<void> {
if (isOpen) {
toggle();
return;
}
try { try {
const { openBrowserSecurely, shouldLaunchBrowser } = await import( const { openBrowserSecurely, shouldLaunchBrowser } = await import(
'@google/gemini-cli-core' '@google/gemini-cli-core'
@@ -228,11 +238,14 @@ export async function toggleDevToolsPanel(
if (shouldLaunchBrowser()) { if (shouldLaunchBrowser()) {
try { try {
await openBrowserSecurely(url); await openBrowserSecurely(url);
// Browser opened successfully, don't open drawer.
return;
} catch (e) { } catch (e) {
debugLogger.warn('Failed to open browser securely:', e); debugLogger.warn('Failed to open browser securely:', e);
} }
} }
toggle(); // If we can't launch browser or it failed, open drawer.
setOpen();
} catch (e) { } catch (e) {
setOpen(); setOpen();
debugLogger.error('Failed to start DevTools server:', e); debugLogger.error('Failed to start DevTools server:', e);