From cea1a867b6abac36d29ddf22b434ae6fffd5d630 Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Mon, 29 Sep 2025 14:19:19 -0700 Subject: [PATCH] Extension update confirm dialog (#10183) --- packages/cli/src/config/extension.test.ts | 6 +- packages/cli/src/config/extension.ts | 50 +++++--- packages/cli/src/ui/AppContainer.tsx | 21 +++- .../cli/src/ui/commands/extensionsCommand.ts | 10 +- packages/cli/src/ui/commands/types.ts | 7 +- .../src/ui/components/ConsentPrompt.test.tsx | 119 ++++++++++++++++++ .../cli/src/ui/components/ConsentPrompt.tsx | 51 ++++++++ .../cli/src/ui/components/DialogManager.tsx | 37 +++--- .../cli/src/ui/contexts/UIStateContext.tsx | 1 + .../cli/src/ui/hooks/slashCommandProcessor.ts | 4 + .../cli/src/ui/hooks/useExtensionUpdates.ts | 43 ++++++- .../cli/src/ui/layouts/DefaultAppLayout.tsx | 5 +- .../src/ui/layouts/ScreenReaderAppLayout.tsx | 5 +- .../src/ui/noninteractive/nonInteractiveUi.ts | 1 + 14 files changed, 310 insertions(+), 50 deletions(-) create mode 100644 packages/cli/src/ui/components/ConsentPrompt.test.tsx create mode 100644 packages/cli/src/ui/components/ConsentPrompt.tsx diff --git a/packages/cli/src/config/extension.test.ts b/packages/cli/src/config/extension.test.ts index 932524d3ac..adede30060 100644 --- a/packages/cli/src/config/extension.test.ts +++ b/packages/cli/src/config/extension.test.ts @@ -898,8 +898,8 @@ describe('extension tests', () => { ).resolves.toBe('my-local-extension'); expect(consoleInfoSpy).toHaveBeenCalledWith( - `Extensions may introduce unexpected behavior. -Ensure you have investigated the extension source and trust the author. + `Installing extension "my-local-extension". +**Extensions may introduce unexpected behavior. Ensure you have investigated the extension source and trust the author.** This extension will run the following MCP servers: * test-server (local): node server.js * test-server-2 (remote): https://google.com`, @@ -954,7 +954,7 @@ This extension will run the following MCP servers: { source: sourceExtDir, type: 'local' }, requestConsentNonInteractive, ), - ).rejects.toThrow('Installation cancelled.'); + ).rejects.toThrow('Installation cancelled for "my-local-extension".'); expect(mockQuestion).toHaveBeenCalledWith( expect.stringContaining('Do you want to continue? [Y/n]: '), diff --git a/packages/cli/src/config/extension.ts b/packages/cli/src/config/extension.ts index 39204a317c..4d8b6c22bc 100644 --- a/packages/cli/src/config/extension.ts +++ b/packages/cli/src/config/extension.ts @@ -37,8 +37,8 @@ import { } from './extensions/github.js'; import type { LoadExtensionContext } from './extensions/variableSchema.js'; import { ExtensionEnablementManager } from './extensions/extensionEnablement.js'; -import type { UseHistoryManagerReturn } from '../ui/hooks/useHistoryManager.js'; import chalk from 'chalk'; +import type { ConfirmationRequest } from '../ui/types.js'; export const EXTENSIONS_DIRECTORY_NAME = path.join(GEMINI_DIR, 'extensions'); @@ -347,7 +347,7 @@ export async function requestConsentNonInteractive( consentDescription: string, ): Promise { console.info(consentDescription); - const result = await promptForContinuationNonInteractive( + const result = await promptForConsentNonInteractive( 'Do you want to continue? [Y/n]: ', ); return result; @@ -359,20 +359,17 @@ export async function requestConsentNonInteractive( * This should not be called from non-interactive mode as it will not work. * * @param consentDescription The description of the thing they will be consenting to. + * @param setExtensionUpdateConfirmationRequest A function to actually add a prompt to the UI. * @returns boolean, whether they consented or not. */ export async function requestConsentInteractive( - _consentDescription: string, - addHistoryItem: UseHistoryManagerReturn['addItem'], + consentDescription: string, + addExtensionUpdateConfirmationRequest: (value: ConfirmationRequest) => void, ): Promise { - addHistoryItem( - { - type: 'info', - text: 'Tried to update an extension but it has some changes that require consent, please use `gemini extensions update`.', - }, - Date.now(), + return await promptForConsentInteractive( + consentDescription + '\n\nDo you want to continue?', + addExtensionUpdateConfirmationRequest, ); - return false; } /** @@ -383,7 +380,7 @@ export async function requestConsentInteractive( * @param prompt A yes/no prompt to ask the user * @returns Whether or not the user answers 'y' (yes). Defaults to 'yes' on enter. */ -async function promptForContinuationNonInteractive( +async function promptForConsentNonInteractive( prompt: string, ): Promise { const readline = await import('node:readline'); @@ -400,6 +397,29 @@ async function promptForContinuationNonInteractive( }); } +/** + * Asks users an interactive yes/no prompt. + * + * This should not be called from non-interactive mode as it will break the CLI. + * + * @param prompt A markdown prompt to ask the user + * @param setExtensionUpdateConfirmationRequest Function to update the UI state with the confirmation request. + * @returns Whether or not the user answers yes. + */ +async function promptForConsentInteractive( + prompt: string, + addExtensionUpdateConfirmationRequest: (value: ConfirmationRequest) => void, +): Promise { + return await new Promise((resolve) => { + addExtensionUpdateConfirmationRequest({ + prompt, + onConfirm: (resolvedConfirmed) => { + resolve(resolvedConfirmed); + }, + }); + }); +} + export async function installExtension( installMetadata: ExtensionInstallMetadata, requestConsent: (consent: string) => Promise, @@ -548,9 +568,9 @@ export async function installExtension( function extensionConsentString(extensionConfig: ExtensionConfig): string { const output: string[] = []; const mcpServerEntries = Object.entries(extensionConfig.mcpServers || {}); - output.push('Extensions may introduce unexpected behavior.'); + output.push(`Installing extension "${extensionConfig.name}".`); output.push( - 'Ensure you have investigated the extension source and trust the author.', + '**Extensions may introduce unexpected behavior. Ensure you have investigated the extension source and trust the author.**', ); if (mcpServerEntries.length) { @@ -600,7 +620,7 @@ async function maybeRequestConsentOrFail( } } if (!(await requestConsent(extensionConsent))) { - throw new Error('Installation cancelled.'); + throw new Error(`Installation cancelled for "${extensionConfig.name}".`); } } diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index f2ed95fc2f..d3e526ac25 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -153,12 +153,16 @@ export const AppContainer = (props: AppContainerProps) => { ); const extensions = config.getExtensions(); - const { extensionsUpdateState, setExtensionsUpdateState } = - useExtensionUpdates( - extensions, - historyManager.addItem, - config.getWorkingDir(), - ); + const { + extensionsUpdateState, + setExtensionsUpdateState, + confirmUpdateExtensionRequests, + addConfirmUpdateExtensionRequest, + } = useExtensionUpdates( + extensions, + historyManager.addItem, + config.getWorkingDir(), + ); const [isPermissionsDialogOpen, setPermissionsDialogOpen] = useState(false); const openPermissionsDialog = useCallback( @@ -456,6 +460,7 @@ Logging in with Google... Please restart Gemini CLI to continue. setDebugMessage, toggleCorgiMode: () => setCorgiMode((prev) => !prev), setExtensionsUpdateState, + addConfirmUpdateExtensionRequest, }), [ setAuthState, @@ -469,6 +474,7 @@ Logging in with Google... Please restart Gemini CLI to continue. setCorgiMode, setExtensionsUpdateState, openPermissionsDialog, + addConfirmUpdateExtensionRequest, ], ); @@ -1037,6 +1043,7 @@ Logging in with Google... Please restart Gemini CLI to continue. isFolderTrustDialogOpen || !!shellConfirmationRequest || !!confirmationRequest || + confirmUpdateExtensionRequests.length > 0 || !!loopDetectionConfirmationRequest || isThemeDialogOpen || isSettingsDialogOpen || @@ -1078,6 +1085,7 @@ Logging in with Google... Please restart Gemini CLI to continue. commandContext, shellConfirmationRequest, confirmationRequest, + confirmUpdateExtensionRequests, loopDetectionConfirmationRequest, geminiMdFileCount, streamingState, @@ -1156,6 +1164,7 @@ Logging in with Google... Please restart Gemini CLI to continue. commandContext, shellConfirmationRequest, confirmationRequest, + confirmUpdateExtensionRequests, loopDetectionConfirmationRequest, geminiMdFileCount, streamingState, diff --git a/packages/cli/src/ui/commands/extensionsCommand.ts b/packages/cli/src/ui/commands/extensionsCommand.ts index 37d70d8bde..037ae1f94e 100644 --- a/packages/cli/src/ui/commands/extensionsCommand.ts +++ b/packages/cli/src/ui/commands/extensionsCommand.ts @@ -54,7 +54,10 @@ async function updateAction(context: CommandContext, args: string) { context.services.config!.getWorkingDir(), // We don't have the ability to prompt for consent yet in this flow. (description) => - requestConsentInteractive(description, context.ui.addItem), + requestConsentInteractive( + description, + context.ui.addConfirmUpdateExtensionRequest, + ), context.services.config!.getExtensions(), context.ui.extensionsUpdateState, context.ui.setExtensionsUpdateState, @@ -80,7 +83,10 @@ async function updateAction(context: CommandContext, args: string) { extension, workingDir, (description) => - requestConsentInteractive(description, context.ui.addItem), + requestConsentInteractive( + description, + context.ui.addConfirmUpdateExtensionRequest, + ), context.ui.extensionsUpdateState.get(extension.name) ?? ExtensionUpdateState.UNKNOWN, (updateState) => { diff --git a/packages/cli/src/ui/commands/types.ts b/packages/cli/src/ui/commands/types.ts index 1d40fe0e52..f6187aabf4 100644 --- a/packages/cli/src/ui/commands/types.ts +++ b/packages/cli/src/ui/commands/types.ts @@ -6,7 +6,11 @@ import type { Dispatch, ReactNode, SetStateAction } from 'react'; import type { Content, PartListUnion } from '@google/genai'; -import type { HistoryItemWithoutId, HistoryItem } from '../types.js'; +import type { + HistoryItemWithoutId, + HistoryItem, + ConfirmationRequest, +} from '../types.js'; import type { Config, GitService, Logger } from '@google/gemini-cli-core'; import type { LoadedSettings } from '../../config/settings.js'; import type { UseHistoryManagerReturn } from '../hooks/useHistoryManager.js'; @@ -66,6 +70,7 @@ export interface CommandContext { setExtensionsUpdateState: Dispatch< SetStateAction> >; + addConfirmUpdateExtensionRequest: (value: ConfirmationRequest) => void; }; // Session-specific data session: { diff --git a/packages/cli/src/ui/components/ConsentPrompt.test.tsx b/packages/cli/src/ui/components/ConsentPrompt.test.tsx new file mode 100644 index 0000000000..4cbd2b1b8e --- /dev/null +++ b/packages/cli/src/ui/components/ConsentPrompt.test.tsx @@ -0,0 +1,119 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { Text } from 'ink'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render } from 'ink-testing-library'; +import { ConsentPrompt } from './ConsentPrompt.js'; +import { RadioButtonSelect } from './shared/RadioButtonSelect.js'; +import { MarkdownDisplay } from '../utils/MarkdownDisplay.js'; + +vi.mock('./shared/RadioButtonSelect.js', () => ({ + RadioButtonSelect: vi.fn(() => null), +})); + +vi.mock('../utils/MarkdownDisplay.js', () => ({ + MarkdownDisplay: vi.fn(() => null), +})); + +const MockedRadioButtonSelect = vi.mocked(RadioButtonSelect); +const MockedMarkdownDisplay = vi.mocked(MarkdownDisplay); + +describe('ConsentPrompt', () => { + const onConfirm = vi.fn(); + const terminalWidth = 80; + + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('renders a string prompt with MarkdownDisplay', () => { + const prompt = 'Are you sure?'; + render( + , + ); + + expect(MockedMarkdownDisplay).toHaveBeenCalledWith( + { + isPending: true, + text: prompt, + terminalWidth, + }, + undefined, + ); + }); + + it('renders a ReactNode prompt directly', () => { + const prompt = Are you sure?; + const { lastFrame } = render( + , + ); + + expect(MockedMarkdownDisplay).not.toHaveBeenCalled(); + expect(lastFrame()).toContain('Are you sure?'); + }); + + it('calls onConfirm with true when "Yes" is selected', () => { + const prompt = 'Are you sure?'; + render( + , + ); + + const onSelect = MockedRadioButtonSelect.mock.calls[0][0].onSelect; + onSelect(true); + + expect(onConfirm).toHaveBeenCalledWith(true); + }); + + it('calls onConfirm with false when "No" is selected', () => { + const prompt = 'Are you sure?'; + render( + , + ); + + const onSelect = MockedRadioButtonSelect.mock.calls[0][0].onSelect; + onSelect(false); + + expect(onConfirm).toHaveBeenCalledWith(false); + }); + + it('passes correct items to RadioButtonSelect', () => { + const prompt = 'Are you sure?'; + render( + , + ); + + expect(MockedRadioButtonSelect).toHaveBeenCalledWith( + expect.objectContaining({ + items: [ + { label: 'Yes', value: true, key: 'Yes' }, + { label: 'No', value: false, key: 'No' }, + ], + }), + undefined, + ); + }); +}); diff --git a/packages/cli/src/ui/components/ConsentPrompt.tsx b/packages/cli/src/ui/components/ConsentPrompt.tsx new file mode 100644 index 0000000000..efa6b136a3 --- /dev/null +++ b/packages/cli/src/ui/components/ConsentPrompt.tsx @@ -0,0 +1,51 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { Box } from 'ink'; +import { type ReactNode } from 'react'; +import { theme } from '../semantic-colors.js'; +import { MarkdownDisplay } from '../utils/MarkdownDisplay.js'; +import { RadioButtonSelect } from './shared/RadioButtonSelect.js'; + +type ConsentPromptProps = { + // If a simple string is given, it will render using markdown by default. + prompt: ReactNode; + onConfirm: (value: boolean) => void; + terminalWidth: number; +}; + +export const ConsentPrompt = (props: ConsentPromptProps) => { + const { prompt, onConfirm, terminalWidth } = props; + + return ( + + {typeof prompt === 'string' ? ( + + ) : ( + prompt + )} + + + + + ); +}; diff --git a/packages/cli/src/ui/components/DialogManager.tsx b/packages/cli/src/ui/components/DialogManager.tsx index 23106dc696..7af65aac58 100644 --- a/packages/cli/src/ui/components/DialogManager.tsx +++ b/packages/cli/src/ui/components/DialogManager.tsx @@ -9,7 +9,7 @@ import { IdeIntegrationNudge } from '../IdeIntegrationNudge.js'; import { LoopDetectionConfirmation } from './LoopDetectionConfirmation.js'; import { FolderTrustDialog } from './FolderTrustDialog.js'; import { ShellConfirmationDialog } from './ShellConfirmationDialog.js'; -import { RadioButtonSelect } from './shared/RadioButtonSelect.js'; +import { ConsentPrompt } from './ConsentPrompt.js'; import { ThemeDialog } from './ThemeDialog.js'; import { SettingsDialog } from './SettingsDialog.js'; import { AuthInProgress } from '../auth/AuthInProgress.js'; @@ -31,10 +31,14 @@ import { IdeTrustChangeDialog } from './IdeTrustChangeDialog.js'; interface DialogManagerProps { addItem: UseHistoryManagerReturn['addItem']; + terminalWidth: number; } // Props for DialogManager -export const DialogManager = ({ addItem }: DialogManagerProps) => { +export const DialogManager = ({ + addItem, + terminalWidth, +}: DialogManagerProps) => { const config = useConfig(); const settings = useSettings(); @@ -94,20 +98,21 @@ export const DialogManager = ({ addItem }: DialogManagerProps) => { } if (uiState.confirmationRequest) { return ( - - {uiState.confirmationRequest.prompt} - - { - uiState.confirmationRequest!.onConfirm(value); - }} - /> - - + + ); + } + if (uiState.confirmUpdateExtensionRequests.length > 0) { + const request = uiState.confirmUpdateExtensionRequests[0]; + return ( + ); } if (uiState.isThemeDialogOpen) { diff --git a/packages/cli/src/ui/contexts/UIStateContext.tsx b/packages/cli/src/ui/contexts/UIStateContext.tsx index 5d58c0a8da..9698fccee6 100644 --- a/packages/cli/src/ui/contexts/UIStateContext.tsx +++ b/packages/cli/src/ui/contexts/UIStateContext.tsx @@ -61,6 +61,7 @@ export interface UIState { commandContext: CommandContext; shellConfirmationRequest: ShellConfirmationRequest | null; confirmationRequest: ConfirmationRequest | null; + confirmUpdateExtensionRequests: ConfirmationRequest[]; loopDetectionConfirmationRequest: LoopDetectionConfirmationRequest | null; geminiMdFileCount: number; streamingState: StreamingState; diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.ts b/packages/cli/src/ui/hooks/slashCommandProcessor.ts index 2ba291d2ad..e317ce68d4 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.ts +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.ts @@ -32,6 +32,7 @@ import type { HistoryItemWithoutId, SlashCommandProcessorResult, HistoryItem, + ConfirmationRequest, } from '../types.js'; import { MessageType } from '../types.js'; import type { LoadedSettings } from '../../config/settings.js'; @@ -57,6 +58,7 @@ interface SlashCommandProcessorActions { setExtensionsUpdateState: Dispatch< SetStateAction> >; + addConfirmUpdateExtensionRequest: (request: ConfirmationRequest) => void; } /** @@ -206,6 +208,8 @@ export const useSlashCommandProcessor = ( reloadCommands, extensionsUpdateState, setExtensionsUpdateState: actions.setExtensionsUpdateState, + addConfirmUpdateExtensionRequest: + actions.addConfirmUpdateExtensionRequest, }, session: { stats: session.stats, diff --git a/packages/cli/src/ui/hooks/useExtensionUpdates.ts b/packages/cli/src/ui/hooks/useExtensionUpdates.ts index 70f77bf15d..8862631225 100644 --- a/packages/cli/src/ui/hooks/useExtensionUpdates.ts +++ b/packages/cli/src/ui/hooks/useExtensionUpdates.ts @@ -7,9 +7,9 @@ import type { GeminiCLIExtension } from '@google/gemini-cli-core'; import { getErrorMessage } from '../../utils/errors.js'; import { ExtensionUpdateState } from '../state/extensions.js'; -import { useState } from 'react'; +import { useCallback, useState } from 'react'; import type { UseHistoryManagerReturn } from './useHistoryManager.js'; -import { MessageType } from '../types.js'; +import { MessageType, type ConfirmationRequest } from '../types.js'; import { checkForAllExtensionUpdates, updateExtension, @@ -25,6 +25,29 @@ export const useExtensionUpdates = ( new Map(), ); const [isChecking, setIsChecking] = useState(false); + const [confirmUpdateExtensionRequests, setConfirmUpdateExtensionRequests] = + useState< + Array<{ + prompt: React.ReactNode; + onConfirm: (confirmed: boolean) => void; + }> + >([]); + const addConfirmUpdateExtensionRequest = useCallback( + (original: ConfirmationRequest) => { + const wrappedRequest = { + prompt: original.prompt, + onConfirm: (confirmed: boolean) => { + // Remove it from the outstanding list of requests by identity. + setConfirmUpdateExtensionRequests((prev) => + prev.filter((r) => r !== wrappedRequest), + ); + original.onConfirm(confirmed); + }, + }; + setConfirmUpdateExtensionRequests((prev) => [...prev, wrappedRequest]); + }, + [setConfirmUpdateExtensionRequests], + ); (async () => { if (isChecking) return; @@ -49,7 +72,11 @@ export const useExtensionUpdates = ( updateExtension( extension, cwd, - (description) => requestConsentInteractive(description, addItem), + (description) => + requestConsentInteractive( + description, + addConfirmUpdateExtensionRequest, + ), currentState, (newState) => { setExtensionsUpdateState((prev) => { @@ -70,8 +97,12 @@ export const useExtensionUpdates = ( ); }) .catch((error) => { - console.error( - `Error updating extension "${extension.name}": ${getErrorMessage(error)}.`, + addItem( + { + type: MessageType.ERROR, + text: getErrorMessage(error), + }, + Date.now(), ); }); } else { @@ -96,5 +127,7 @@ export const useExtensionUpdates = ( return { extensionsUpdateState, setExtensionsUpdateState, + confirmUpdateExtensionRequests, + addConfirmUpdateExtensionRequest, }; }; diff --git a/packages/cli/src/ui/layouts/DefaultAppLayout.tsx b/packages/cli/src/ui/layouts/DefaultAppLayout.tsx index d07b1fe48d..1fd980e387 100644 --- a/packages/cli/src/ui/layouts/DefaultAppLayout.tsx +++ b/packages/cli/src/ui/layouts/DefaultAppLayout.tsx @@ -24,7 +24,10 @@ export const DefaultAppLayout: React.FC = () => { {uiState.dialogsVisible ? ( - + ) : ( )} diff --git a/packages/cli/src/ui/layouts/ScreenReaderAppLayout.tsx b/packages/cli/src/ui/layouts/ScreenReaderAppLayout.tsx index 36b75820fb..b25646bb32 100644 --- a/packages/cli/src/ui/layouts/ScreenReaderAppLayout.tsx +++ b/packages/cli/src/ui/layouts/ScreenReaderAppLayout.tsx @@ -25,7 +25,10 @@ export const ScreenReaderAppLayout: React.FC = () => { {uiState.dialogsVisible ? ( - + ) : ( )} diff --git a/packages/cli/src/ui/noninteractive/nonInteractiveUi.ts b/packages/cli/src/ui/noninteractive/nonInteractiveUi.ts index 4e53aae57f..bdc7ca4461 100644 --- a/packages/cli/src/ui/noninteractive/nonInteractiveUi.ts +++ b/packages/cli/src/ui/noninteractive/nonInteractiveUi.ts @@ -25,5 +25,6 @@ export function createNonInteractiveUI(): CommandContext['ui'] { reloadCommands: () => {}, extensionsUpdateState: new Map(), setExtensionsUpdateState: (_updateState) => {}, + addConfirmUpdateExtensionRequest: (_request) => {}, }; }