From f5dd1068f65d3ceb66e2de7b97a84cfcce102da8 Mon Sep 17 00:00:00 2001 From: Dmitry Lyalin Date: Wed, 11 Feb 2026 11:30:46 -0500 Subject: [PATCH 1/8] fix(core): complete MCP discovery when configured servers are skipped (#18586) Co-authored-by: christine betts Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- .../core/src/tools/mcp-client-manager.test.ts | 37 +++++++++++++++++++ packages/core/src/tools/mcp-client-manager.ts | 9 +++++ 2 files changed, 46 insertions(+) diff --git a/packages/core/src/tools/mcp-client-manager.test.ts b/packages/core/src/tools/mcp-client-manager.test.ts index bbab5ef12d..352c2c12f0 100644 --- a/packages/core/src/tools/mcp-client-manager.test.ts +++ b/packages/core/src/tools/mcp-client-manager.test.ts @@ -103,6 +103,43 @@ describe('McpClientManager', () => { expect(manager.getDiscoveryState()).toBe(MCPDiscoveryState.COMPLETED); }); + it('should mark discovery completed when all configured servers are user-disabled', async () => { + mockConfig.getMcpServers.mockReturnValue({ + 'test-server': {}, + }); + mockConfig.getMcpEnablementCallbacks.mockReturnValue({ + isSessionDisabled: vi.fn().mockReturnValue(false), + isFileEnabled: vi.fn().mockResolvedValue(false), + }); + + const manager = new McpClientManager('0.0.1', toolRegistry, mockConfig); + const promise = manager.startConfiguredMcpServers(); + expect(manager.getDiscoveryState()).toBe(MCPDiscoveryState.IN_PROGRESS); + await promise; + + expect(manager.getDiscoveryState()).toBe(MCPDiscoveryState.COMPLETED); + expect(manager.getMcpServerCount()).toBe(0); + expect(mockedMcpClient.connect).not.toHaveBeenCalled(); + expect(mockedMcpClient.discover).not.toHaveBeenCalled(); + }); + + it('should mark discovery completed when all configured servers are blocked', async () => { + mockConfig.getMcpServers.mockReturnValue({ + 'test-server': {}, + }); + mockConfig.getBlockedMcpServers.mockReturnValue(['test-server']); + + const manager = new McpClientManager('0.0.1', toolRegistry, mockConfig); + const promise = manager.startConfiguredMcpServers(); + expect(manager.getDiscoveryState()).toBe(MCPDiscoveryState.IN_PROGRESS); + await promise; + + expect(manager.getDiscoveryState()).toBe(MCPDiscoveryState.COMPLETED); + expect(manager.getMcpServerCount()).toBe(0); + expect(mockedMcpClient.connect).not.toHaveBeenCalled(); + expect(mockedMcpClient.discover).not.toHaveBeenCalled(); + }); + it('should not discover tools if folder is not trusted', async () => { mockConfig.getMcpServers.mockReturnValue({ 'test-server': {}, diff --git a/packages/core/src/tools/mcp-client-manager.ts b/packages/core/src/tools/mcp-client-manager.ts index b38b00616b..a6783788e8 100644 --- a/packages/core/src/tools/mcp-client-manager.ts +++ b/packages/core/src/tools/mcp-client-manager.ts @@ -337,6 +337,15 @@ export class McpClientManager { this.maybeDiscoverMcpServer(name, config), ), ); + + // If every configured server was skipped (for example because all are + // disabled by user settings), no discovery promise is created. In that + // case we must still mark discovery complete or the UI will wait forever. + if (this.discoveryState === MCPDiscoveryState.IN_PROGRESS) { + this.discoveryState = MCPDiscoveryState.COMPLETED; + this.eventEmitter?.emit('mcp-client-update', this.clients); + } + await this.cliConfig.refreshMcpContext(); } From 34a47a51f47ea271cb32a8a3fc0e5b8ea11c25d8 Mon Sep 17 00:00:00 2001 From: Sehoon Shon Date: Wed, 11 Feb 2026 12:01:50 -0500 Subject: [PATCH 2/8] fix(core): cache CLI version to ensure consistency during sessions (#18793) --- .../core/src/core/contentGenerator.test.ts | 2 ++ packages/core/src/utils/version.test.ts | 20 ++++++++++++++++++- packages/core/src/utils/version.ts | 19 +++++++++++++++--- 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/packages/core/src/core/contentGenerator.test.ts b/packages/core/src/core/contentGenerator.test.ts index 74e20e73d4..9b7c3ac802 100644 --- a/packages/core/src/core/contentGenerator.test.ts +++ b/packages/core/src/core/contentGenerator.test.ts @@ -18,6 +18,7 @@ import { LoggingContentGenerator } from './loggingContentGenerator.js'; import { loadApiKey } from './apiKeyCredentialStorage.js'; import { FakeContentGenerator } from './fakeContentGenerator.js'; import { RecordingContentGenerator } from './recordingContentGenerator.js'; +import { resetVersionCache } from '../utils/version.js'; vi.mock('../code_assist/codeAssist.js'); vi.mock('@google/genai'); @@ -35,6 +36,7 @@ const mockConfig = { describe('createContentGenerator', () => { beforeEach(() => { + resetVersionCache(); vi.clearAllMocks(); }); diff --git a/packages/core/src/utils/version.test.ts b/packages/core/src/utils/version.test.ts index 775860b629..94a6f5c7c9 100644 --- a/packages/core/src/utils/version.test.ts +++ b/packages/core/src/utils/version.test.ts @@ -5,7 +5,7 @@ */ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import { getVersion } from './version.js'; +import { getVersion, resetVersionCache } from './version.js'; import { getPackageJson } from './package.js'; vi.mock('./package.js', () => ({ @@ -17,6 +17,8 @@ describe('version', () => { beforeEach(() => { vi.resetModules(); + vi.clearAllMocks(); + resetVersionCache(); process.env = { ...originalEnv }; vi.mocked(getPackageJson).mockResolvedValue({ version: '1.0.0' }); }); @@ -43,4 +45,20 @@ describe('version', () => { const version = await getVersion(); expect(version).toBe('unknown'); }); + + it('should cache the version and only call getPackageJson once', async () => { + delete process.env['CLI_VERSION']; + vi.mocked(getPackageJson).mockResolvedValue({ version: '1.2.3' }); + + const version1 = await getVersion(); + expect(version1).toBe('1.2.3'); + expect(getPackageJson).toHaveBeenCalledTimes(1); + + // Change the mock value to simulate an update on disk + vi.mocked(getPackageJson).mockResolvedValue({ version: '2.0.0' }); + + const version2 = await getVersion(); + expect(version2).toBe('1.2.3'); // Should still be the cached version + expect(getPackageJson).toHaveBeenCalledTimes(1); // Should not have been called again + }); }); diff --git a/packages/core/src/utils/version.ts b/packages/core/src/utils/version.ts index ecd7a8b503..53e8b7d569 100644 --- a/packages/core/src/utils/version.ts +++ b/packages/core/src/utils/version.ts @@ -11,7 +11,20 @@ import path from 'node:path'; const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); -export async function getVersion(): Promise { - const pkgJson = await getPackageJson(__dirname); - return process.env['CLI_VERSION'] || pkgJson?.version || 'unknown'; +let versionPromise: Promise | undefined; + +export function getVersion(): Promise { + if (versionPromise) { + return versionPromise; + } + versionPromise = (async () => { + const pkgJson = await getPackageJson(__dirname); + return process.env['CLI_VERSION'] || pkgJson?.version || 'unknown'; + })(); + return versionPromise; +} + +/** For testing purposes only */ +export function resetVersionCache(): void { + versionPromise = undefined; } From 0080589939352ce84a0c7f59b0592d19f9ae11eb Mon Sep 17 00:00:00 2001 From: Brad Dux <959674+braddux@users.noreply.github.com> Date: Wed, 11 Feb 2026 09:29:18 -0800 Subject: [PATCH 3/8] fix(cli): resolve double rendering in shpool and address vscode lint warnings (#18704) --- packages/cli/src/gemini.test.tsx | 29 ++++++--------- packages/cli/src/gemini.tsx | 53 +++++++++++++++++----------- packages/cli/src/ui/AppContainer.tsx | 4 ++- packages/core/src/config/config.ts | 4 +++ 4 files changed, 50 insertions(+), 40 deletions(-) diff --git a/packages/cli/src/gemini.test.tsx b/packages/cli/src/gemini.test.tsx index 41f9978d7c..2e55c9b25d 100644 --- a/packages/cli/src/gemini.test.tsx +++ b/packages/cli/src/gemini.test.tsx @@ -238,18 +238,15 @@ vi.mock('./validateNonInterActiveAuth.js', () => ({ })); describe('gemini.tsx main function', () => { - let originalEnvGeminiSandbox: string | undefined; - let originalEnvSandbox: string | undefined; let originalIsTTY: boolean | undefined; let initialUnhandledRejectionListeners: NodeJS.UnhandledRejectionListener[] = []; beforeEach(() => { // Store and clear sandbox-related env variables to ensure a consistent test environment - originalEnvGeminiSandbox = process.env['GEMINI_SANDBOX']; - originalEnvSandbox = process.env['SANDBOX']; - delete process.env['GEMINI_SANDBOX']; - delete process.env['SANDBOX']; + vi.stubEnv('GEMINI_SANDBOX', ''); + vi.stubEnv('SANDBOX', ''); + vi.stubEnv('SHPOOL_SESSION_NAME', ''); initialUnhandledRejectionListeners = process.listeners('unhandledRejection'); @@ -260,18 +257,6 @@ describe('gemini.tsx main function', () => { }); afterEach(() => { - // Restore original env variables - if (originalEnvGeminiSandbox !== undefined) { - process.env['GEMINI_SANDBOX'] = originalEnvGeminiSandbox; - } else { - delete process.env['GEMINI_SANDBOX']; - } - if (originalEnvSandbox !== undefined) { - process.env['SANDBOX'] = originalEnvSandbox; - } else { - delete process.env['SANDBOX']; - } - const currentListeners = process.listeners('unhandledRejection'); currentListeners.forEach((listener) => { if (!initialUnhandledRejectionListeners.includes(listener)) { @@ -282,6 +267,7 @@ describe('gemini.tsx main function', () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any (process.stdin as any).isTTY = originalIsTTY; + vi.unstubAllEnvs(); vi.restoreAllMocks(); }); @@ -1209,7 +1195,12 @@ describe('startInteractiveUI', () => { registerTelemetryConfig: vi.fn(), })); + beforeEach(() => { + vi.stubEnv('SHPOOL_SESSION_NAME', ''); + }); + afterEach(() => { + vi.unstubAllEnvs(); vi.restoreAllMocks(); }); @@ -1308,7 +1299,7 @@ describe('startInteractiveUI', () => { // Verify all startup tasks were called expect(getVersion).toHaveBeenCalledTimes(1); - expect(registerCleanup).toHaveBeenCalledTimes(3); + expect(registerCleanup).toHaveBeenCalledTimes(4); // Verify cleanup handler is registered with unmount function const cleanupFn = vi.mocked(registerCleanup).mock.calls[0][0]; diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index 68ce4c99b6..a18f3ace37 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -57,8 +57,8 @@ import { writeToStderr, disableMouseEvents, enableMouseEvents, - enterAlternateScreen, disableLineWrapping, + enableLineWrapping, shouldEnterAlternateScreen, startupProfiler, ExitCodes, @@ -89,6 +89,7 @@ import { SessionStatsProvider } from './ui/contexts/SessionContext.js'; import { VimModeProvider } from './ui/contexts/VimModeContext.js'; import { KeypressProvider } from './ui/contexts/KeypressContext.js'; import { useKittyKeyboardProtocol } from './ui/hooks/useKittyKeyboardProtocol.js'; +import { useTerminalSize } from './ui/hooks/useTerminalSize.js'; import { relaunchAppInChildProcess, relaunchOnExitCode, @@ -214,9 +215,13 @@ export async function startInteractiveUI( const { stdout: inkStdout, stderr: inkStderr } = createWorkingStdio(); + const isShpool = !!process.env['SHPOOL_SESSION_NAME']; + // Create wrapper component to use hooks inside render const AppWrapper = () => { useKittyKeyboardProtocol(); + const { columns, rows } = useTerminalSize(); + return ( setTimeout(resolve, 100)); + } + const instance = render( process.env['DEBUG'] ? ( @@ -273,10 +290,19 @@ export async function startInteractiveUI( patchConsole: false, alternateBuffer: useAlternateBuffer, incrementalRendering: - settings.merged.ui.incrementalRendering !== false && useAlternateBuffer, + settings.merged.ui.incrementalRendering !== false && + useAlternateBuffer && + !isShpool, }, ); + if (useAlternateBuffer) { + disableLineWrapping(); + registerCleanup(() => { + enableLineWrapping(); + }); + } + checkForUpdates(settings) .then((info) => { handleAutoUpdate(info, settings, config.getProjectRoot()); @@ -590,26 +616,13 @@ export async function main() { // input showing up in the output. process.stdin.setRawMode(true); - if ( - shouldEnterAlternateScreen( - isAlternateBufferEnabled(settings), - config.getScreenReader(), - ) - ) { - enterAlternateScreen(); - disableLineWrapping(); - - // Ink will cleanup so there is no need for us to manually cleanup. - } - // This cleanup isn't strictly needed but may help in certain situations. - const restoreRawMode = () => { + process.on('SIGTERM', () => { process.stdin.setRawMode(wasRaw); - }; - process.off('SIGTERM', restoreRawMode); - process.on('SIGTERM', restoreRawMode); - process.off('SIGINT', restoreRawMode); - process.on('SIGINT', restoreRawMode); + }); + process.on('SIGINT', () => { + process.stdin.setRawMode(wasRaw); + }); } await setupTerminalAndTheme(config, settings); diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index 7d863a638f..97e1cec2b7 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -363,7 +363,9 @@ export const AppContainer = (props: AppContainerProps) => { (async () => { // Note: the program will not work if this fails so let errors be // handled by the global catch. - await config.initialize(); + if (!config.isInitialized()) { + await config.initialize(); + } setConfigInitialized(true); startupProfiler.flush(config); diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 1570339010..6d811799bc 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -905,6 +905,10 @@ export class Config { ); } + isInitialized(): boolean { + return this.initialized; + } + /** * Must only be called once, throws if called again. */ From 65d26e73a290c74bf9db1f34b8f9865e25a62392 Mon Sep 17 00:00:00 2001 From: Jerop Kipruto Date: Wed, 11 Feb 2026 12:32:02 -0500 Subject: [PATCH 4/8] feat(plan): document and validate Plan Mode policy overrides (#18825) --- docs/cli/plan-mode.md | 50 ++++++++++++ docs/core/policy-engine.md | 16 +++- packages/core/src/policy/config.test.ts | 103 ++++++++++++++++++++++++ 3 files changed, 166 insertions(+), 3 deletions(-) diff --git a/docs/cli/plan-mode.md b/docs/cli/plan-mode.md index 0d6b72206e..105e5aa5e7 100644 --- a/docs/cli/plan-mode.md +++ b/docs/cli/plan-mode.md @@ -30,6 +30,7 @@ implementation strategy. - [The Planning Workflow](#the-planning-workflow) - [Exiting Plan Mode](#exiting-plan-mode) - [Tool Restrictions](#tool-restrictions) + - [Customizing Policies](#customizing-policies) ## Starting in Plan Mode @@ -98,6 +99,53 @@ These are the only allowed tools: - **Planning (Write):** [`write_file`] and [`replace`] ONLY allowed for `.md` files in the `~/.gemini/tmp//plans/` directory. +### Customizing Policies + +Plan Mode is designed to be read-only by default to ensure safety during the +research phase. However, you may occasionally need to allow specific tools to +assist in your planning. + +Because user policies (Tier 2) have a higher base priority than built-in +policies (Tier 1), you can override Plan Mode's default restrictions by creating +a rule in your `~/.gemini/policies/` directory. + +#### Example: Allow `git status` and `git diff` in Plan Mode + +This rule allows you to check the repository status and see changes while in +Plan Mode. + +`~/.gemini/policies/git-research.toml` + +```toml +[[rule]] +toolName = "run_shell_command" +commandPrefix = ["git status", "git diff"] +decision = "allow" +priority = 100 +modes = ["plan"] +``` + +#### Example: Enable research sub-agents in Plan Mode + +You can enable [experimental research sub-agents] like `codebase_investigator` +to help gather architecture details during the planning phase. + +`~/.gemini/policies/research-subagents.toml` + +```toml +[[rule]] +toolName = "codebase_investigator" +decision = "allow" +priority = 100 +modes = ["plan"] +``` + +Tell the agent it can use these tools in your prompt, for example: _"You can +check ongoing changes in git."_ + +For more information on how the policy engine works, see the [Policy Engine +Guide]. + [`list_directory`]: /docs/tools/file-system.md#1-list_directory-readfolder [`read_file`]: /docs/tools/file-system.md#2-read_file-readfile [`grep_search`]: /docs/tools/file-system.md#5-grep_search-searchtext @@ -106,3 +154,5 @@ These are the only allowed tools: [`google_web_search`]: /docs/tools/web-search.md [`replace`]: /docs/tools/file-system.md#6-replace-edit [MCP tools]: /docs/tools/mcp-server.md +[experimental research sub-agents]: /docs/core/subagents.md +[Policy Engine Guide]: /docs/core/policy-engine.md diff --git a/docs/core/policy-engine.md b/docs/core/policy-engine.md index f09ca01b70..a99a6652d8 100644 --- a/docs/core/policy-engine.md +++ b/docs/core/policy-engine.md @@ -119,9 +119,17 @@ For example: Approval modes allow the policy engine to apply different sets of rules based on the CLI's operational mode. A rule can be associated with one or more modes -(e.g., `yolo`, `autoEdit`). The rule will only be active if the CLI is running -in one of its specified modes. If a rule has no modes specified, it is always -active. +(e.g., `yolo`, `autoEdit`, `plan`). The rule will only be active if the CLI is +running in one of its specified modes. If a rule has no modes specified, it is +always active. + +- `default`: The standard interactive mode where most write tools require + confirmation. +- `autoEdit`: Optimized for automated code editing; some write tools may be + auto-approved. +- `plan`: A strict, read-only mode for research and design. See [Customizing + Plan Mode Policies]. +- `yolo`: A mode where all tools are auto-approved (use with extreme caution). ## Rule matching @@ -303,3 +311,5 @@ out-of-the-box experience. - In **`yolo`** mode, a high-priority rule allows all tools. - In **`autoEdit`** mode, rules allow certain write operations to happen without prompting. + +[Customizing Plan Mode Policies]: /docs/cli/plan-mode.md#customizing-policies diff --git a/packages/core/src/policy/config.test.ts b/packages/core/src/policy/config.test.ts index 25f7e4a150..620cdd8500 100644 --- a/packages/core/src/policy/config.test.ts +++ b/packages/core/src/policy/config.test.ts @@ -951,4 +951,107 @@ name = "invalid-name" vi.doUnmock('node:fs/promises'); }); + + it('should allow overriding Plan Mode deny with user policy', async () => { + const actualFs = + await vi.importActual( + 'node:fs/promises', + ); + + const mockReaddir = vi.fn( + async ( + path: string | Buffer | URL, + options?: Parameters[1], + ) => { + const normalizedPath = nodePath.normalize(path.toString()); + if (normalizedPath.includes(nodePath.normalize('.gemini/policies'))) { + return [ + { + name: 'user-plan.toml', + isFile: () => true, + isDirectory: () => false, + }, + ] as unknown as Awaited>; + } + return actualFs.readdir( + path, + options as Parameters[1], + ); + }, + ); + + const mockReadFile = vi.fn( + async ( + path: Parameters[0], + options: Parameters[1], + ) => { + const normalizedPath = nodePath.normalize(path.toString()); + if (normalizedPath.includes('user-plan.toml')) { + return ` +[[rule]] +toolName = "run_shell_command" +commandPrefix = ["git status", "git diff"] +decision = "allow" +priority = 100 +modes = ["plan"] + +[[rule]] +toolName = "codebase_investigator" +decision = "allow" +priority = 100 +modes = ["plan"] +`; + } + return actualFs.readFile(path, options); + }, + ); + + vi.doMock('node:fs/promises', () => ({ + ...actualFs, + default: { ...actualFs, readFile: mockReadFile, readdir: mockReaddir }, + readFile: mockReadFile, + readdir: mockReaddir, + })); + + vi.resetModules(); + const { createPolicyEngineConfig } = await import('./config.js'); + + const settings: PolicySettings = {}; + const config = await createPolicyEngineConfig( + settings, + ApprovalMode.PLAN, + nodePath.join(__dirname, 'policies'), + ); + + const shellRules = config.rules?.filter( + (r) => + r.toolName === 'run_shell_command' && + r.decision === PolicyDecision.ALLOW && + r.modes?.includes(ApprovalMode.PLAN) && + r.argsPattern, + ); + expect(shellRules).toHaveLength(2); + expect( + shellRules?.some((r) => r.argsPattern?.test('{"command":"git status"}')), + ).toBe(true); + expect( + shellRules?.some((r) => r.argsPattern?.test('{"command":"git diff"}')), + ).toBe(true); + expect( + shellRules?.every( + (r) => !r.argsPattern?.test('{"command":"git commit"}'), + ), + ).toBe(true); + + const subagentRule = config.rules?.find( + (r) => + r.toolName === 'codebase_investigator' && + r.decision === PolicyDecision.ALLOW && + r.modes?.includes(ApprovalMode.PLAN), + ); + expect(subagentRule).toBeDefined(); + expect(subagentRule?.priority).toBeCloseTo(2.1, 5); + + vi.doUnmock('node:fs/promises'); + }); }); From eb9223b6a44de83ff849be9ea5730e14581a1320 Mon Sep 17 00:00:00 2001 From: Jacob Richman Date: Wed, 11 Feb 2026 09:38:01 -0800 Subject: [PATCH 5/8] Fix pressing any key to exit select mode. (#18421) --- packages/cli/src/ui/AppContainer.test.tsx | 228 ++++++++---------- packages/cli/src/ui/AppContainer.tsx | 23 +- .../cli/src/ui/contexts/KeypressContext.tsx | 88 +++++-- packages/cli/src/ui/hooks/useKeypress.ts | 15 +- 4 files changed, 193 insertions(+), 161 deletions(-) diff --git a/packages/cli/src/ui/AppContainer.test.tsx b/packages/cli/src/ui/AppContainer.test.tsx index b5b512434e..b6fdd53325 100644 --- a/packages/cli/src/ui/AppContainer.test.tsx +++ b/packages/cli/src/ui/AppContainer.test.tsx @@ -84,7 +84,7 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => { }; }); import ansiEscapes from 'ansi-escapes'; -import { type LoadedSettings, mergeSettings } from '../config/settings.js'; +import { mergeSettings, type LoadedSettings } from '../config/settings.js'; import type { InitializationResult } from '../core/initializer.js'; import { useQuotaAndFallback } from './hooks/useQuotaAndFallback.js'; import { UIStateContext, type UIState } from './contexts/UIStateContext.js'; @@ -92,6 +92,7 @@ import { UIActionsContext, type UIActions, } from './contexts/UIActionsContext.js'; +import { KeypressProvider } from './contexts/KeypressContext.js'; // Mock useStdout to capture terminal title writes vi.mock('ink', async (importOriginal) => { @@ -133,7 +134,6 @@ vi.mock('./hooks/useGeminiStream.js'); vi.mock('./hooks/vim.js'); vi.mock('./hooks/useFocus.js'); vi.mock('./hooks/useBracketedPaste.js'); -vi.mock('./hooks/useKeypress.js'); vi.mock('./hooks/useLoadingIndicator.js'); vi.mock('./hooks/useFolderTrust.js'); vi.mock('./hooks/useIdeTrustListener.js'); @@ -197,7 +197,7 @@ import { useTextBuffer } from './components/shared/text-buffer.js'; import { useLogger } from './hooks/useLogger.js'; import { useLoadingIndicator } from './hooks/useLoadingIndicator.js'; import { useInputHistoryStore } from './hooks/useInputHistoryStore.js'; -import { useKeypress, type Key } from './hooks/useKeypress.js'; +import { useKeypress } from './hooks/useKeypress.js'; import { measureElement } from 'ink'; import { useTerminalSize } from './hooks/useTerminalSize.js'; import { @@ -232,13 +232,15 @@ describe('AppContainer State Management', () => { resumedSessionData?: ResumedSessionData; } = {}) => ( - + + + ); @@ -268,7 +270,6 @@ describe('AppContainer State Management', () => { const mockedUseTextBuffer = useTextBuffer as Mock; const mockedUseLogger = useLogger as Mock; const mockedUseLoadingIndicator = useLoadingIndicator as Mock; - const mockedUseKeypress = useKeypress as Mock; const mockedUseInputHistoryStore = useInputHistoryStore as Mock; const mockedUseHookDisplayState = useHookDisplayState as Mock; const mockedUseTerminalTheme = useTerminalTheme as Mock; @@ -1770,47 +1771,36 @@ describe('AppContainer State Management', () => { }); describe('Keyboard Input Handling (CTRL+C / CTRL+D)', () => { - let handleGlobalKeypress: (key: Key) => boolean; let mockHandleSlashCommand: Mock; let mockCancelOngoingRequest: Mock; let rerender: () => void; let unmount: () => void; + let stdin: ReturnType['stdin']; // Helper function to reduce boilerplate in tests const setupKeypressTest = async () => { const renderResult = renderAppContainer(); + stdin = renderResult.stdin; await act(async () => { vi.advanceTimersByTime(0); }); - rerender = () => renderResult.rerender(getAppContainer()); + rerender = () => { + renderResult.rerender(getAppContainer()); + }; unmount = renderResult.unmount; }; - const pressKey = (key: Partial, times = 1) => { + const pressKey = (sequence: string, times = 1) => { for (let i = 0; i < times; i++) { act(() => { - handleGlobalKeypress({ - name: 'c', - shift: false, - alt: false, - ctrl: false, - cmd: false, - ...key, - } as Key); + stdin.write(sequence); }); rerender(); } }; beforeEach(() => { - // Capture the keypress handler from the AppContainer - mockedUseKeypress.mockImplementation( - (callback: (key: Key) => boolean) => { - handleGlobalKeypress = callback; - }, - ); - // Mock slash command handler mockHandleSlashCommand = vi.fn(); mockedUseSlashCommandProcessor.mockReturnValue({ @@ -1855,7 +1845,7 @@ describe('AppContainer State Management', () => { }); await setupKeypressTest(); - pressKey({ name: 'c', ctrl: true }); + pressKey('\x03'); // Ctrl+C expect(mockCancelOngoingRequest).toHaveBeenCalledTimes(1); expect(mockHandleSlashCommand).not.toHaveBeenCalled(); @@ -1865,7 +1855,7 @@ describe('AppContainer State Management', () => { it('should quit on second press', async () => { await setupKeypressTest(); - pressKey({ name: 'c', ctrl: true }, 2); + pressKey('\x03', 2); // Ctrl+C expect(mockCancelOngoingRequest).toHaveBeenCalledTimes(2); expect(mockHandleSlashCommand).toHaveBeenCalledWith( @@ -1880,7 +1870,7 @@ describe('AppContainer State Management', () => { it('should reset press count after a timeout', async () => { await setupKeypressTest(); - pressKey({ name: 'c', ctrl: true }); + pressKey('\x03'); // Ctrl+C expect(mockHandleSlashCommand).not.toHaveBeenCalled(); // Advance timer past the reset threshold @@ -1888,7 +1878,7 @@ describe('AppContainer State Management', () => { vi.advanceTimersByTime(WARNING_PROMPT_DURATION_MS + 1); }); - pressKey({ name: 'c', ctrl: true }); + pressKey('\x03'); // Ctrl+C expect(mockHandleSlashCommand).not.toHaveBeenCalled(); unmount(); }); @@ -1898,7 +1888,7 @@ describe('AppContainer State Management', () => { it('should quit on second press if buffer is empty', async () => { await setupKeypressTest(); - pressKey({ name: 'd', ctrl: true }, 2); + pressKey('\x04', 2); // Ctrl+D expect(mockHandleSlashCommand).toHaveBeenCalledWith( '/quit', @@ -1909,7 +1899,7 @@ describe('AppContainer State Management', () => { unmount(); }); - it('should NOT quit if buffer is not empty (bubbles from InputPrompt)', async () => { + it('should NOT quit if buffer is not empty', async () => { mockedUseTextBuffer.mockReturnValue({ text: 'some text', setText: vi.fn(), @@ -1919,30 +1909,12 @@ describe('AppContainer State Management', () => { }); await setupKeypressTest(); - // Capture return value - let result = true; - const originalPressKey = (key: Partial) => { - act(() => { - result = handleGlobalKeypress({ - name: 'd', - shift: false, - alt: false, - ctrl: true, - cmd: false, - ...key, - } as Key); - }); - rerender(); - }; + pressKey('\x04'); // Ctrl+D - originalPressKey({ name: 'd', ctrl: true }); - - // AppContainer's handler should return true if it reaches it - expect(result).toBe(true); - // But it should only be called once, so count is 1, not quitting yet. + // Should only be called once, so count is 1, not quitting yet. expect(mockHandleSlashCommand).not.toHaveBeenCalled(); - originalPressKey({ name: 'd', ctrl: true }); + pressKey('\x04'); // Ctrl+D // Now count is 2, it should quit. expect(mockHandleSlashCommand).toHaveBeenCalledWith( '/quit', @@ -1956,7 +1928,7 @@ describe('AppContainer State Management', () => { it('should reset press count after a timeout', async () => { await setupKeypressTest(); - pressKey({ name: 'd', ctrl: true }); + pressKey('\x04'); // Ctrl+D expect(mockHandleSlashCommand).not.toHaveBeenCalled(); // Advance timer past the reset threshold @@ -1964,7 +1936,7 @@ describe('AppContainer State Management', () => { vi.advanceTimersByTime(WARNING_PROMPT_DURATION_MS + 1); }); - pressKey({ name: 'd', ctrl: true }); + pressKey('\x04'); // Ctrl+D expect(mockHandleSlashCommand).not.toHaveBeenCalled(); unmount(); }); @@ -1982,7 +1954,7 @@ describe('AppContainer State Management', () => { it('should focus shell input on Tab', async () => { await setupKeypressTest(); - pressKey({ name: 'tab', shift: false }); + pressKey('\t'); expect(capturedUIState.embeddedShellFocused).toBe(true); unmount(); @@ -1992,11 +1964,11 @@ describe('AppContainer State Management', () => { await setupKeypressTest(); // Focus first - pressKey({ name: 'tab', shift: false }); + pressKey('\t'); expect(capturedUIState.embeddedShellFocused).toBe(true); // Unfocus via Shift+Tab - pressKey({ name: 'tab', shift: true }); + pressKey('\x1b[Z'); expect(capturedUIState.embeddedShellFocused).toBe(false); unmount(); }); @@ -2015,13 +1987,7 @@ describe('AppContainer State Management', () => { // Focus it act(() => { - handleGlobalKeypress({ - name: 'tab', - shift: false, - alt: false, - ctrl: false, - cmd: false, - } as Key); + renderResult.stdin.write('\t'); }); expect(capturedUIState.embeddedShellFocused).toBe(true); @@ -2056,7 +2022,7 @@ describe('AppContainer State Management', () => { expect(capturedUIState.embeddedShellFocused).toBe(false); // Press Tab - pressKey({ name: 'tab', shift: false }); + pressKey('\t'); // Should be focused expect(capturedUIState.embeddedShellFocused).toBe(true); @@ -2084,7 +2050,7 @@ describe('AppContainer State Management', () => { expect(capturedUIState.embeddedShellFocused).toBe(false); // Press Ctrl+B - pressKey({ name: 'b', ctrl: true }); + pressKey('\x02'); // Should have toggled (closed) the shell expect(mockToggleBackgroundShell).toHaveBeenCalled(); @@ -2113,7 +2079,7 @@ describe('AppContainer State Management', () => { }); // Press Ctrl+B - pressKey({ name: 'b', ctrl: true }); + pressKey('\x02'); // Should have toggled (shown) the shell expect(mockToggleBackgroundShell).toHaveBeenCalled(); @@ -2126,11 +2092,14 @@ describe('AppContainer State Management', () => { }); describe('Copy Mode (CTRL+S)', () => { - let handleGlobalKeypress: (key: Key) => boolean; let rerender: () => void; let unmount: () => void; + let stdin: ReturnType['stdin']; - const setupCopyModeTest = async (isAlternateMode = false) => { + const setupCopyModeTest = async ( + isAlternateMode = false, + childHandler?: Mock, + ) => { // Update settings for this test run const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true); const testSettings = { @@ -2144,23 +2113,39 @@ describe('AppContainer State Management', () => { }, } as unknown as LoadedSettings; - const renderResult = renderAppContainer({ settings: testSettings }); + function TestChild() { + useKeypress(childHandler || (() => {}), { + isActive: !!childHandler, + priority: true, + }); + return null; + } + + const getTree = (settings: LoadedSettings) => ( + + + + + + + ); + + const renderResult = render(getTree(testSettings)); + stdin = renderResult.stdin; await act(async () => { vi.advanceTimersByTime(0); }); - rerender = () => - renderResult.rerender(getAppContainer({ settings: testSettings })); + rerender = () => renderResult.rerender(getTree(testSettings)); unmount = renderResult.unmount; }; beforeEach(() => { mocks.mockStdout.write.mockClear(); - mockedUseKeypress.mockImplementation( - (callback: (key: Key) => boolean) => { - handleGlobalKeypress = callback; - }, - ); vi.useFakeTimers(); }); @@ -2186,15 +2171,7 @@ describe('AppContainer State Management', () => { mocks.mockStdout.write.mockClear(); // Clear initial enable call act(() => { - handleGlobalKeypress({ - name: 's', - shift: false, - alt: false, - ctrl: true, - cmd: false, - insertable: false, - sequence: '\x13', - }); + stdin.write('\x13'); // Ctrl+S }); rerender(); @@ -2213,30 +2190,14 @@ describe('AppContainer State Management', () => { // Turn it on (disable mouse) act(() => { - handleGlobalKeypress({ - name: 's', - shift: false, - alt: false, - ctrl: true, - cmd: false, - insertable: false, - sequence: '\x13', - }); + stdin.write('\x13'); // Ctrl+S }); rerender(); expect(disableMouseEvents).toHaveBeenCalled(); // Turn it off (enable mouse) act(() => { - handleGlobalKeypress({ - name: 'any', // Any key should exit copy mode - shift: false, - alt: false, - ctrl: false, - cmd: false, - insertable: true, - sequence: 'a', - }); + stdin.write('a'); // Any key should exit copy mode }); rerender(); @@ -2249,15 +2210,7 @@ describe('AppContainer State Management', () => { // Enter copy mode act(() => { - handleGlobalKeypress({ - name: 's', - shift: false, - alt: false, - ctrl: true, - cmd: false, - insertable: false, - sequence: '\x13', - }); + stdin.write('\x13'); // Ctrl+S }); rerender(); @@ -2265,15 +2218,7 @@ describe('AppContainer State Management', () => { // Press any other key act(() => { - handleGlobalKeypress({ - name: 'a', - shift: false, - alt: false, - ctrl: false, - cmd: false, - insertable: true, - sequence: 'a', - }); + stdin.write('a'); }); rerender(); @@ -2281,6 +2226,37 @@ describe('AppContainer State Management', () => { expect(enableMouseEvents).toHaveBeenCalled(); unmount(); }); + + it('should have higher priority than other priority listeners when enabled', async () => { + // 1. Initial state with a child component's priority listener (already subscribed) + // It should NOT handle Ctrl+S so we can enter copy mode. + const childHandler = vi.fn().mockReturnValue(false); + await setupCopyModeTest(true, childHandler); + + // 2. Enter copy mode + act(() => { + stdin.write('\x13'); // Ctrl+S + }); + rerender(); + + // 3. Verify we are in copy mode + expect(disableMouseEvents).toHaveBeenCalled(); + + // 4. Press any key + childHandler.mockClear(); + // Now childHandler should return true for other keys, simulating a greedy listener + childHandler.mockReturnValue(true); + + act(() => { + stdin.write('a'); + }); + rerender(); + + // 5. Verify that the exit handler took priority and childHandler was NOT called + expect(childHandler).not.toHaveBeenCalled(); + expect(enableMouseEvents).toHaveBeenCalled(); + unmount(); + }); } }); }); diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index 97e1cec2b7..174510d066 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -101,6 +101,7 @@ import { type LoadableSettingScope, SettingScope } from '../config/settings.js'; import { type InitializationResult } from '../core/initializer.js'; import { useFocus } from './hooks/useFocus.js'; import { useKeypress, type Key } from './hooks/useKeypress.js'; +import { KeypressPriority } from './contexts/KeypressContext.js'; import { keyMatchers, Command } from './keyMatchers.js'; import { useLoadingIndicator } from './hooks/useLoadingIndicator.js'; import { useShellInactivityStatus } from './hooks/useShellInactivityStatus.js'; @@ -1483,13 +1484,6 @@ Logging in with Google... Restarting Gemini CLI to continue. const handleGlobalKeypress = useCallback( (key: Key): boolean => { - if (copyModeEnabled) { - setCopyModeEnabled(false); - enableMouseEvents(); - // We don't want to process any other keys if we're in copy mode. - return true; - } - // Debug log keystrokes if enabled if (settings.merged.general.debugKeystrokeLogging) { debugLogger.log('[DEBUG] Keystroke:', JSON.stringify(key)); @@ -1656,7 +1650,6 @@ Logging in with Google... Restarting Gemini CLI to continue. settings.merged.general.debugKeystrokeLogging, refreshStatic, setCopyModeEnabled, - copyModeEnabled, isAlternateBuffer, backgroundCurrentShell, toggleBackgroundShell, @@ -1672,6 +1665,20 @@ Logging in with Google... Restarting Gemini CLI to continue. useKeypress(handleGlobalKeypress, { isActive: true, priority: true }); + useKeypress( + () => { + setCopyModeEnabled(false); + enableMouseEvents(); + return true; + }, + { + isActive: copyModeEnabled, + // We need to receive keypresses first so they do not bubble to other + // handlers. + priority: KeypressPriority.Critical, + }, + ); + useEffect(() => { // Respect hideWindowTitle settings if (settings.merged.ui.hideWindowTitle) return; diff --git a/packages/cli/src/ui/contexts/KeypressContext.tsx b/packages/cli/src/ui/contexts/KeypressContext.tsx index 6b3a7db6d9..d31b1e4fbd 100644 --- a/packages/cli/src/ui/contexts/KeypressContext.tsx +++ b/packages/cli/src/ui/contexts/KeypressContext.tsx @@ -6,6 +6,7 @@ import { debugLogger, type Config } from '@google/gemini-cli-core'; import { useStdin } from 'ink'; +import { MultiMap } from 'mnemonist'; import type React from 'react'; import { createContext, @@ -26,6 +27,13 @@ export const ESC_TIMEOUT = 50; export const PASTE_TIMEOUT = 30_000; export const FAST_RETURN_TIMEOUT = 30; +export enum KeypressPriority { + Low = -100, + Normal = 0, + High = 100, + Critical = 200, +} + // Parse the key itself const KEY_INFO_MAP: Record< string, @@ -645,7 +653,10 @@ export interface Key { export type KeypressHandler = (key: Key) => boolean | void; interface KeypressContextValue { - subscribe: (handler: KeypressHandler, priority?: boolean) => void; + subscribe: ( + handler: KeypressHandler, + priority?: KeypressPriority | boolean, + ) => void; unsubscribe: (handler: KeypressHandler) => void; } @@ -674,44 +685,75 @@ export function KeypressProvider({ }) { const { stdin, setRawMode } = useStdin(); - const prioritySubscribers = useRef>(new Set()).current; - const normalSubscribers = useRef>(new Set()).current; + const subscribersToPriority = useRef>( + new Map(), + ).current; + const subscribers = useRef( + new MultiMap(Set), + ).current; + const sortedPriorities = useRef([]); const subscribe = useCallback( - (handler: KeypressHandler, priority = false) => { - const set = priority ? prioritySubscribers : normalSubscribers; - set.add(handler); + ( + handler: KeypressHandler, + priority: KeypressPriority | boolean = KeypressPriority.Normal, + ) => { + const p = + typeof priority === 'boolean' + ? priority + ? KeypressPriority.High + : KeypressPriority.Normal + : priority; + + subscribersToPriority.set(handler, p); + const hadPriority = subscribers.has(p); + subscribers.set(p, handler); + + if (!hadPriority) { + // Cache sorted priorities only when a new priority level is added + sortedPriorities.current = Array.from(subscribers.keys()).sort( + (a, b) => b - a, + ); + } }, - [prioritySubscribers, normalSubscribers], + [subscribers, subscribersToPriority], ); const unsubscribe = useCallback( (handler: KeypressHandler) => { - prioritySubscribers.delete(handler); - normalSubscribers.delete(handler); + const p = subscribersToPriority.get(handler); + if (p !== undefined) { + subscribers.remove(p, handler); + subscribersToPriority.delete(handler); + + if (!subscribers.has(p)) { + // Cache sorted priorities only when a priority level is completely removed + sortedPriorities.current = Array.from(subscribers.keys()).sort( + (a, b) => b - a, + ); + } + } }, - [prioritySubscribers, normalSubscribers], + [subscribers, subscribersToPriority], ); const broadcast = useCallback( (key: Key) => { - // Process priority subscribers first, in reverse order (stack behavior: last subscribed is first to handle) - const priorityHandlers = Array.from(prioritySubscribers).reverse(); - for (const handler of priorityHandlers) { - if (handler(key) === true) { - return; - } - } + // Use cached sorted priorities to avoid sorting on every keypress + for (const p of sortedPriorities.current) { + const set = subscribers.get(p); + if (!set) continue; - // Then process normal subscribers, also in reverse order - const normalHandlers = Array.from(normalSubscribers).reverse(); - for (const handler of normalHandlers) { - if (handler(key) === true) { - return; + // Within a priority level, use stack behavior (last subscribed is first to handle) + const handlers = Array.from(set).reverse(); + for (const handler of handlers) { + if (handler(key) === true) { + return; + } } } }, - [prioritySubscribers, normalSubscribers], + [subscribers], ); useEffect(() => { diff --git a/packages/cli/src/ui/hooks/useKeypress.ts b/packages/cli/src/ui/hooks/useKeypress.ts index 7df1b195a6..a06599d2d2 100644 --- a/packages/cli/src/ui/hooks/useKeypress.ts +++ b/packages/cli/src/ui/hooks/useKeypress.ts @@ -5,8 +5,12 @@ */ import { useEffect } from 'react'; -import type { KeypressHandler, Key } from '../contexts/KeypressContext.js'; -import { useKeypressContext } from '../contexts/KeypressContext.js'; +import { + useKeypressContext, + type KeypressHandler, + type Key, + type KeypressPriority, +} from '../contexts/KeypressContext.js'; export type { Key }; @@ -16,11 +20,14 @@ export type { Key }; * @param onKeypress - The callback function to execute on each keypress. * @param options - Options to control the hook's behavior. * @param options.isActive - Whether the hook should be actively listening for input. - * @param options.priority - Whether the hook should have priority over normal subscribers. + * @param options.priority - Priority level (integer or KeypressPriority enum) or boolean for backward compatibility. */ export function useKeypress( onKeypress: KeypressHandler, - { isActive, priority }: { isActive: boolean; priority?: boolean }, + { + isActive, + priority, + }: { isActive: boolean; priority?: KeypressPriority | boolean }, ) { const { subscribe, unsubscribe } = useKeypressContext(); From b19b026f30b5a939c32a3b4372b5697f98d97654 Mon Sep 17 00:00:00 2001 From: Sandy Tao Date: Wed, 11 Feb 2026 09:45:43 -0800 Subject: [PATCH 6/8] fix(cli): update F12 behavior to only open drawer if browser fails (#18829) --- packages/cli/src/ui/AppContainer.tsx | 2 ++ .../cli/src/utils/devtoolsService.test.ts | 36 ++++++++++++------- packages/cli/src/utils/devtoolsService.ts | 19 ++++++++-- 3 files changed, 42 insertions(+), 15 deletions(-) diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index 174510d066..72fdb0ce48 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -1521,6 +1521,7 @@ Logging in with Google... Restarting Gemini CLI to continue. ); await toggleDevToolsPanel( config, + showErrorDetails, () => setShowErrorDetails((prev) => !prev), () => setShowErrorDetails(true), ); @@ -1660,6 +1661,7 @@ Logging in with Google... Restarting Gemini CLI to continue. tabFocusTimeoutRef, showTransientMessage, settings.merged.general.devtools, + showErrorDetails, ], ); diff --git a/packages/cli/src/utils/devtoolsService.test.ts b/packages/cli/src/utils/devtoolsService.test.ts index 922d4d1483..cb3b907d97 100644 --- a/packages/cli/src/utils/devtoolsService.test.ts +++ b/packages/cli/src/utils/devtoolsService.test.ts @@ -437,7 +437,19 @@ describe('devtoolsService', () => { }); 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 toggle = vi.fn(); const setOpen = vi.fn(); @@ -447,18 +459,18 @@ describe('devtoolsService', () => { mockDevToolsInstance.start.mockResolvedValue('http://127.0.0.1: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)); MockWebSocket.instances[0].simulateError(); await promise; - expect(toggle).toHaveBeenCalledTimes(1); + expect(toggle).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 toggle = vi.fn(); const setOpen = vi.fn(); @@ -468,18 +480,18 @@ describe('devtoolsService', () => { mockDevToolsInstance.start.mockResolvedValue('http://127.0.0.1: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)); MockWebSocket.instances[0].simulateError(); await promise; - expect(toggle).toHaveBeenCalledTimes(1); - expect(setOpen).not.toHaveBeenCalled(); + expect(toggle).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 toggle = vi.fn(); const setOpen = vi.fn(); @@ -488,15 +500,15 @@ describe('devtoolsService', () => { mockDevToolsInstance.start.mockResolvedValue('http://127.0.0.1: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)); MockWebSocket.instances[0].simulateError(); await promise; - expect(toggle).toHaveBeenCalledTimes(1); - expect(setOpen).not.toHaveBeenCalled(); + expect(toggle).not.toHaveBeenCalled(); + expect(setOpen).toHaveBeenCalledTimes(1); }); it('calls setOpen when DevTools server fails to start', async () => { @@ -506,7 +518,7 @@ describe('devtoolsService', () => { 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)); MockWebSocket.instances[0].simulateError(); diff --git a/packages/cli/src/utils/devtoolsService.ts b/packages/cli/src/utils/devtoolsService.ts index 35abf0ec96..5e4b7710c4 100644 --- a/packages/cli/src/utils/devtoolsService.ts +++ b/packages/cli/src/utils/devtoolsService.ts @@ -212,14 +212,24 @@ async function startDevToolsServerImpl(config: Config): Promise { /** * Handles the F12 key toggle for the DevTools panel. - * Starts the DevTools server, attempts to open the browser, - * and always calls the toggle callback regardless of the outcome. + * Starts the DevTools server, attempts to open the browser. + * 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( config: Config, + isOpen: boolean, toggle: () => void, setOpen: () => void, ): Promise { + if (isOpen) { + toggle(); + return; + } + try { const { openBrowserSecurely, shouldLaunchBrowser } = await import( '@google/gemini-cli-core' @@ -228,11 +238,14 @@ export async function toggleDevToolsPanel( if (shouldLaunchBrowser()) { try { await openBrowserSecurely(url); + // Browser opened successfully, don't open drawer. + return; } catch (e) { debugLogger.warn('Failed to open browser securely:', e); } } - toggle(); + // If we can't launch browser or it failed, open drawer. + setOpen(); } catch (e) { setOpen(); debugLogger.error('Failed to start DevTools server:', e); From 84ce53aafab4374fb3cd24de49f10624fa56cbad Mon Sep 17 00:00:00 2001 From: Adib234 <30782825+Adib234@users.noreply.github.com> Date: Wed, 11 Feb 2026 12:59:03 -0500 Subject: [PATCH 7/8] feat(plan): allow skills to be enabled in plan mode (#18817) Co-authored-by: Jerop Kipruto --- docs/cli/plan-mode.md | 24 +++++++++++ packages/core/src/policy/policies/plan.toml | 4 +- .../core/src/policy/policy-engine.test.ts | 40 +++++++++++++++++++ packages/core/src/tools/tool-names.ts | 1 + 4 files changed, 67 insertions(+), 2 deletions(-) diff --git a/docs/cli/plan-mode.md b/docs/cli/plan-mode.md index 105e5aa5e7..0fe46698c9 100644 --- a/docs/cli/plan-mode.md +++ b/docs/cli/plan-mode.md @@ -30,6 +30,7 @@ implementation strategy. - [The Planning Workflow](#the-planning-workflow) - [Exiting Plan Mode](#exiting-plan-mode) - [Tool Restrictions](#tool-restrictions) + - [Customizing Planning with Skills](#customizing-planning-with-skills) - [Customizing Policies](#customizing-policies) ## Starting in Plan Mode @@ -98,6 +99,28 @@ These are the only allowed tools: `postgres_read_schema`) are allowed. - **Planning (Write):** [`write_file`] and [`replace`] ONLY allowed for `.md` files in the `~/.gemini/tmp//plans/` directory. +- **Skills:** [`activate_skill`] (allows loading specialized instructions and + resources in a read-only manner) + +### Customizing Planning with Skills + +You can leverage [Agent Skills](./skills.md) to customize how Gemini CLI +approaches planning for specific types of tasks. When a skill is activated +during Plan Mode, its specialized instructions and procedural workflows will +guide the research and design phases. + +For example: + +- A **"Database Migration"** skill could ensure the plan includes data safety + checks and rollback strategies. +- A **"Security Audit"** skill could prompt the agent to look for specific + vulnerabilities during codebase exploration. +- A **"Frontend Design"** skill could guide the agent to use specific UI + components and accessibility standards in its proposal. + +To use a skill in Plan Mode, you can explicitly ask the agent to "use the +[skill-name] skill to plan..." or the agent may autonomously activate it based +on the task description. ### Customizing Policies @@ -154,5 +177,6 @@ Guide]. [`google_web_search`]: /docs/tools/web-search.md [`replace`]: /docs/tools/file-system.md#6-replace-edit [MCP tools]: /docs/tools/mcp-server.md +[`activate_skill`]: /docs/cli/skills.md [experimental research sub-agents]: /docs/core/subagents.md [Policy Engine Guide]: /docs/core/policy-engine.md diff --git a/packages/core/src/policy/policies/plan.toml b/packages/core/src/policy/policies/plan.toml index 12aa94d893..656c100845 100644 --- a/packages/core/src/policy/policies/plan.toml +++ b/packages/core/src/policy/policies/plan.toml @@ -31,12 +31,12 @@ decision = "deny" priority = 60 modes = ["plan"] -deny_message = "You are in Plan Mode - adjust your prompt to only use read and search tools." +deny_message = "You are in Plan Mode with access to read-only tools. Execution of scripts (including those from skills) is blocked." # Explicitly Allow Read-Only Tools in Plan mode. [[rule]] -toolName = ["glob", "grep_search", "list_directory", "read_file", "google_web_search"] +toolName = ["glob", "grep_search", "list_directory", "read_file", "google_web_search", "activate_skill"] decision = "allow" priority = 70 modes = ["plan"] diff --git a/packages/core/src/policy/policy-engine.test.ts b/packages/core/src/policy/policy-engine.test.ts index 6c59161af4..59b0fd8106 100644 --- a/packages/core/src/policy/policy-engine.test.ts +++ b/packages/core/src/policy/policy-engine.test.ts @@ -2086,4 +2086,44 @@ describe('PolicyEngine', () => { expect(result.decision).toBe(PolicyDecision.ALLOW); }); }); + + describe('Plan Mode', () => { + it('should allow activate_skill but deny shell commands in Plan Mode', async () => { + const rules: PolicyRule[] = [ + { + decision: PolicyDecision.DENY, + priority: 60, + modes: [ApprovalMode.PLAN], + denyMessage: + 'You are in Plan Mode with access to read-only tools. Execution of scripts (including those from skills) is blocked.', + }, + { + toolName: 'activate_skill', + decision: PolicyDecision.ALLOW, + priority: 70, + modes: [ApprovalMode.PLAN], + }, + ]; + + engine = new PolicyEngine({ + rules, + approvalMode: ApprovalMode.PLAN, + }); + + const skillResult = await engine.check( + { name: 'activate_skill', args: { name: 'test' } }, + undefined, + ); + expect(skillResult.decision).toBe(PolicyDecision.ALLOW); + + const shellResult = await engine.check( + { name: 'run_shell_command', args: { command: 'ls' } }, + undefined, + ); + expect(shellResult.decision).toBe(PolicyDecision.DENY); + expect(shellResult.rule?.denyMessage).toContain( + 'Execution of scripts (including those from skills) is blocked', + ); + }); + }); }); diff --git a/packages/core/src/tools/tool-names.ts b/packages/core/src/tools/tool-names.ts index 70e882ebe1..f837edbe29 100644 --- a/packages/core/src/tools/tool-names.ts +++ b/packages/core/src/tools/tool-names.ts @@ -108,6 +108,7 @@ export const PLAN_MODE_TOOLS = [ LS_TOOL_NAME, WEB_SEARCH_TOOL_NAME, ASK_USER_TOOL_NAME, + ACTIVATE_SKILL_TOOL_NAME, EXIT_PLAN_MODE_TOOL_NAME, ] as const; From 77849cadac89cf190c50380a6db3aab384bb0869 Mon Sep 17 00:00:00 2001 From: Jerop Kipruto Date: Wed, 11 Feb 2026 13:53:43 -0500 Subject: [PATCH 8/8] docs(plan): add documentation for plan mode tools (#18827) --- docs/cli/plan-mode.md | 9 ++++--- docs/tools/index.md | 1 + docs/tools/planning.md | 55 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 3 deletions(-) create mode 100644 docs/tools/planning.md diff --git a/docs/cli/plan-mode.md b/docs/cli/plan-mode.md index 0fe46698c9..6c85515755 100644 --- a/docs/cli/plan-mode.md +++ b/docs/cli/plan-mode.md @@ -63,7 +63,8 @@ You can enter Plan Mode in three ways: 1. **Keyboard Shortcut:** Press `Shift+Tab` to cycle through approval modes (`Default` -> `Plan` -> `Auto-Edit`). 2. **Command:** Type `/plan` in the input box. -3. **Natural Language:** Ask the agent to "start a plan for...". +3. **Natural Language:** Ask the agent to "start a plan for...". The agent will + then call the [`enter_plan_mode`] tool to switch modes. ### The Planning Workflow @@ -83,8 +84,8 @@ You can enter Plan Mode in three ways: To exit Plan Mode: 1. **Keyboard Shortcut:** Press `Shift+Tab` to cycle to the desired mode. -1. **Tool:** The agent calls the `exit_plan_mode` tool to present the finalized - plan for your approval. +2. **Tool:** The agent calls the [`exit_plan_mode`] tool to present the + finalized plan for your approval. ## Tool Restrictions @@ -180,3 +181,5 @@ Guide]. [`activate_skill`]: /docs/cli/skills.md [experimental research sub-agents]: /docs/core/subagents.md [Policy Engine Guide]: /docs/core/policy-engine.md +[`enter_plan_mode`]: /docs/tools/planning.md#1-enter_plan_mode-enterplanmode +[`exit_plan_mode`]: /docs/tools/planning.md#2-exit_plan_mode-exitplanmode diff --git a/docs/tools/index.md b/docs/tools/index.md index c21c3dc610..ff594056ac 100644 --- a/docs/tools/index.md +++ b/docs/tools/index.md @@ -86,6 +86,7 @@ Gemini CLI's built-in tools can be broadly categorized as follows: information across sessions. - **[Todo Tool](./todos.md) (`write_todos`):** For managing subtasks of complex requests. +- **[Planning Tools](./planning.md):** For entering and exiting Plan Mode. Additionally, these tools incorporate: diff --git a/docs/tools/planning.md b/docs/tools/planning.md new file mode 100644 index 0000000000..686b27f058 --- /dev/null +++ b/docs/tools/planning.md @@ -0,0 +1,55 @@ +# Gemini CLI planning tools + +Planning tools allow the Gemini model to switch into a safe, read-only "Plan +Mode" for researching and planning complex changes, and to signal the +finalization of a plan to the user. + +## 1. `enter_plan_mode` (EnterPlanMode) + +`enter_plan_mode` switches the CLI to Plan Mode. This tool is typically called +by the agent when you ask it to "start a plan" using natural language. In this +mode, the agent is restricted to read-only tools to allow for safe exploration +and planning. + +- **Tool name:** `enter_plan_mode` +- **Display name:** Enter Plan Mode +- **File:** `enter-plan-mode.ts` +- **Parameters:** + - `reason` (string, optional): A short reason explaining why the agent is + entering plan mode (e.g., "Starting a complex feature implementation"). +- **Behavior:** + - Switches the CLI's approval mode to `PLAN`. + - Notifies the user that the agent has entered Plan Mode. +- **Output (`llmContent`):** A message indicating the switch, e.g., + `Switching to Plan mode.` +- **Confirmation:** Yes. The user is prompted to confirm entering Plan Mode. + +## 2. `exit_plan_mode` (ExitPlanMode) + +`exit_plan_mode` signals that the planning phase is complete. It presents the +finalized plan to the user and requests approval to start the implementation. + +- **Tool name:** `exit_plan_mode` +- **Display name:** Exit Plan Mode +- **File:** `exit-plan-mode.ts` +- **Parameters:** + - `plan_path` (string, required): The path to the finalized Markdown plan + file. This file MUST be located within the project's temporary plans + directory (e.g., `~/.gemini/tmp//plans/`). +- **Behavior:** + - Validates that the `plan_path` is within the allowed directory and that the + file exists and has content. + - Presents the plan to the user for review. + - If the user approves the plan: + - Switches the CLI's approval mode to the user's chosen approval mode ( + `DEFAULT` or `AUTO_EDIT`). + - Marks the plan as approved for implementation. + - If the user rejects the plan: + - Stays in Plan Mode. + - Returns user feedback to the model to refine the plan. +- **Output (`llmContent`):** + - On approval: A message indicating the plan was approved and the new approval + mode. + - On rejection: A message containing the user's feedback. +- **Confirmation:** Yes. Shows the finalized plan and asks for user approval to + proceed with implementation.