From 62346875e48395f7fb9e595e0dbe3c1dc6dfb5be Mon Sep 17 00:00:00 2001 From: Jerop Kipruto Date: Fri, 30 Jan 2026 13:32:21 -0500 Subject: [PATCH] feat(plan): reuse standard tool confirmation for `AskUser` tool (#17864) Co-authored-by: jacob314 --- packages/cli/src/ui/App.test.tsx | 1 - packages/cli/src/ui/AppContainer.test.tsx | 44 --- packages/cli/src/ui/AppContainer.tsx | 79 +---- .../src/ui/__snapshots__/App.test.tsx.snap | 2 +- .../src/ui/components/AskUserDialog.test.tsx | 74 +++++ .../cli/src/ui/components/AskUserDialog.tsx | 168 +++++----- .../ui/components/BubblingRegression.test.tsx | 2 + .../cli/src/ui/components/DialogManager.tsx | 18 -- .../ui/components/ToolConfirmationQueue.tsx | 52 ++- .../__snapshots__/AskUserDialog.test.tsx.snap | 207 ++++++------ .../ToolConfirmationQueue.test.tsx.snap | 6 +- .../messages/ToolConfirmationMessage.tsx | 99 ++++-- .../shared/BaseSelectionList.test.tsx | 39 +-- .../components/shared/BaseSelectionList.tsx | 4 +- .../BaseSelectionList.test.tsx.snap | 31 ++ ...DescriptiveRadioButtonSelect.test.tsx.snap | 6 +- .../__snapshots__/TableRenderer.test.tsx.snap | 64 ---- .../cli/src/zed-integration/zedIntegration.ts | 3 + packages/core/src/confirmation-bus/types.ts | 12 +- packages/core/src/policy/policies/plan.toml | 2 +- packages/core/src/scheduler/confirmation.ts | 4 + packages/core/src/tools/ask-user.test.ts | 298 ++++++++++-------- packages/core/src/tools/ask-user.ts | 144 +++------ packages/core/src/tools/tools.ts | 18 +- 24 files changed, 675 insertions(+), 702 deletions(-) create mode 100644 packages/cli/src/ui/components/shared/__snapshots__/BaseSelectionList.test.tsx.snap diff --git a/packages/cli/src/ui/App.test.tsx b/packages/cli/src/ui/App.test.tsx index bff645b6f7..bd663ba195 100644 --- a/packages/cli/src/ui/App.test.tsx +++ b/packages/cli/src/ui/App.test.tsx @@ -235,7 +235,6 @@ describe('App', () => { expect(lastFrame()).toContain('Tips for getting started'); expect(lastFrame()).toContain('Notifications'); expect(lastFrame()).toContain('Action Required'); // From ToolConfirmationQueue - expect(lastFrame()).toContain('1 of 1'); expect(lastFrame()).toContain('Composer'); expect(lastFrame()).toMatchSnapshot(); }); diff --git a/packages/cli/src/ui/AppContainer.test.tsx b/packages/cli/src/ui/AppContainer.test.tsx index d897bc91b4..ef0a24cd92 100644 --- a/packages/cli/src/ui/AppContainer.test.tsx +++ b/packages/cli/src/ui/AppContainer.test.tsx @@ -32,13 +32,7 @@ import { UserAccountManager, type ContentGeneratorConfig, type AgentDefinition, - MessageBusType, - QuestionType, } from '@google/gemini-cli-core'; -import { - AskUserActionsContext, - type AskUserState, -} from './contexts/AskUserActionsContext.js'; // Mock coreEvents const mockCoreEvents = vi.hoisted(() => ({ @@ -124,11 +118,9 @@ vi.mock('ink', async (importOriginal) => { // so we can assert against them in our tests. let capturedUIState: UIState; let capturedUIActions: UIActions; -let capturedAskUserRequest: AskUserState | null; function TestContextConsumer() { capturedUIState = useContext(UIStateContext)!; capturedUIActions = useContext(UIActionsContext)!; - capturedAskUserRequest = useContext(AskUserActionsContext)?.request ?? null; return null; } @@ -298,7 +290,6 @@ describe('AppContainer State Management', () => { mocks.mockStdout.write.mockClear(); capturedUIState = null!; - capturedAskUserRequest = null; // **Provide a default return value for EVERY mocked hook.** mockedUseQuotaAndFallback.mockReturnValue({ @@ -2674,41 +2665,6 @@ describe('AppContainer State Management', () => { unmount!(); }); - - it('should show ask user dialog when request is received', async () => { - let unmount: () => void; - await act(async () => { - const result = renderAppContainer(); - unmount = result.unmount; - }); - - const questions = [ - { - question: 'What is your favorite color?', - header: 'Color Preference', - type: QuestionType.TEXT, - }, - ]; - - await act(async () => { - await mockConfig.getMessageBus().publish({ - type: MessageBusType.ASK_USER_REQUEST, - questions, - correlationId: 'test-id', - }); - }); - - await waitFor( - () => { - expect(capturedAskUserRequest).not.toBeNull(); - expect(capturedAskUserRequest?.questions).toEqual(questions); - expect(capturedAskUserRequest?.correlationId).toBe('test-id'); - }, - { timeout: 2000 }, - ); - - unmount!(); - }); }); describe('Regression Tests', () => { diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index aeeff89289..e1d23115ca 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -31,10 +31,6 @@ import { } from './types.js'; import { MessageType, StreamingState } from './types.js'; import { ToolActionsProvider } from './contexts/ToolActionsContext.js'; -import { - AskUserActionsProvider, - type AskUserState, -} from './contexts/AskUserActionsContext.js'; import { type EditorType, type Config, @@ -70,8 +66,6 @@ import { SessionEndReason, generateSummary, type ConsentRequestPayload, - MessageBusType, - type AskUserRequest, type AgentsDiscoveredPayload, ChangeAuthRequestedError, } from '@google/gemini-cli-core'; @@ -344,11 +338,6 @@ export const AppContainer = (props: AppContainerProps) => { AgentDefinition | undefined >(); - // AskUser dialog state - const [askUserRequest, setAskUserRequest] = useState( - null, - ); - const openAgentConfigDialog = useCallback( (name: string, displayName: string, definition: AgentDefinition) => { setSelectedAgentName(name); @@ -366,56 +355,6 @@ export const AppContainer = (props: AppContainerProps) => { setSelectedAgentDefinition(undefined); }, []); - // Subscribe to ASK_USER_REQUEST messages from the message bus - useEffect(() => { - const messageBus = config.getMessageBus(); - - const handler = (msg: AskUserRequest) => { - setAskUserRequest({ - questions: msg.questions, - correlationId: msg.correlationId, - }); - }; - - messageBus.subscribe(MessageBusType.ASK_USER_REQUEST, handler); - - return () => { - messageBus.unsubscribe(MessageBusType.ASK_USER_REQUEST, handler); - }; - }, [config]); - - // Handler to submit ask_user answers - const handleAskUserSubmit = useCallback( - async (answers: { [questionIndex: string]: string }) => { - if (!askUserRequest) return; - - const messageBus = config.getMessageBus(); - await messageBus.publish({ - type: MessageBusType.ASK_USER_RESPONSE, - correlationId: askUserRequest.correlationId, - answers, - }); - - setAskUserRequest(null); - }, - [config, askUserRequest], - ); - - // Handler to cancel ask_user dialog - const handleAskUserCancel = useCallback(async () => { - if (!askUserRequest) return; - - const messageBus = config.getMessageBus(); - await messageBus.publish({ - type: MessageBusType.ASK_USER_RESPONSE, - correlationId: askUserRequest.correlationId, - answers: {}, - cancelled: true, - }); - - setAskUserRequest(null); - }, [config, askUserRequest]); - const toggleDebugProfiler = useCallback( () => setShowDebugProfiler((prev) => !prev), [], @@ -1546,10 +1485,6 @@ Logging in with Google... Restarting Gemini CLI to continue. } if (keyMatchers[Command.QUIT](key)) { - // Skip when ask_user dialog is open (use Esc to cancel instead) - if (askUserRequest) { - return; - } // If the user presses Ctrl+C, we want to cancel any ongoing requests. // This should happen regardless of the count. cancelOngoingRequest?.(); @@ -1694,7 +1629,6 @@ Logging in with Google... Restarting Gemini CLI to continue. setCtrlDPressCount, handleSlashCommand, cancelOngoingRequest, - askUserRequest, activePtyId, embeddedShellFocused, settings.merged.general.debugKeystrokeLogging, @@ -1815,7 +1749,6 @@ Logging in with Google... Restarting Gemini CLI to continue. const nightly = props.version.includes('nightly'); const dialogsVisible = - !!askUserRequest || shouldShowIdePrompt || isFolderTrustDialogOpen || adminSettingsChanged || @@ -2273,15 +2206,9 @@ Logging in with Google... Restarting Gemini CLI to continue. }} > - - - - - + + + diff --git a/packages/cli/src/ui/__snapshots__/App.test.tsx.snap b/packages/cli/src/ui/__snapshots__/App.test.tsx.snap index 2587751008..f8451ee353 100644 --- a/packages/cli/src/ui/__snapshots__/App.test.tsx.snap +++ b/packages/cli/src/ui/__snapshots__/App.test.tsx.snap @@ -125,7 +125,7 @@ Tips for getting started: 4. /help for more information. HistoryItemDisplay ╭──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮ -│ Action Required 1 of 1 │ +│ Action Required │ │ │ │ ? ls list directory │ │ │ diff --git a/packages/cli/src/ui/components/AskUserDialog.test.tsx b/packages/cli/src/ui/components/AskUserDialog.test.tsx index 7ee45f96bd..645321dfc0 100644 --- a/packages/cli/src/ui/components/AskUserDialog.test.tsx +++ b/packages/cli/src/ui/components/AskUserDialog.test.tsx @@ -41,6 +41,8 @@ describe('AskUserDialog', () => { questions={authQuestion} onSubmit={vi.fn()} onCancel={vi.fn()} + width={120} + availableHeight={20} />, { width: 120 }, ); @@ -105,6 +107,8 @@ describe('AskUserDialog', () => { questions={questions} onSubmit={onSubmit} onCancel={vi.fn()} + width={120} + availableHeight={20} />, { width: 120 }, ); @@ -124,6 +128,8 @@ describe('AskUserDialog', () => { questions={authQuestion} onSubmit={onSubmit} onCancel={vi.fn()} + width={120} + availableHeight={20} />, { width: 120 }, ); @@ -153,12 +159,42 @@ describe('AskUserDialog', () => { }); }); + it('shows scroll arrows when options exceed available height', async () => { + const questions: Question[] = [ + { + question: 'Choose an option', + header: 'Scroll Test', + options: Array.from({ length: 15 }, (_, i) => ({ + label: `Option ${i + 1}`, + description: `Description ${i + 1}`, + })), + multiSelect: false, + }, + ]; + + const { lastFrame } = renderWithProviders( + , + ); + + await waitFor(() => { + expect(lastFrame()).toMatchSnapshot(); + }); + }); + it('navigates to custom option when typing unbound characters (Type-to-Jump)', async () => { const { stdin, lastFrame } = renderWithProviders( , { width: 120 }, ); @@ -209,6 +245,8 @@ describe('AskUserDialog', () => { questions={multiQuestions} onSubmit={vi.fn()} onCancel={vi.fn()} + width={120} + availableHeight={20} />, { width: 120 }, ); @@ -222,6 +260,8 @@ describe('AskUserDialog', () => { questions={authQuestion} onSubmit={vi.fn()} onCancel={vi.fn()} + width={120} + availableHeight={20} />, { width: 120 }, ); @@ -235,6 +275,8 @@ describe('AskUserDialog', () => { questions={authQuestion} onSubmit={vi.fn()} onCancel={vi.fn()} + width={120} + availableHeight={20} />, { width: 120 }, ); @@ -265,6 +307,8 @@ describe('AskUserDialog', () => { questions={multiQuestions} onSubmit={vi.fn()} onCancel={vi.fn()} + width={120} + availableHeight={20} />, { width: 120 }, ); @@ -306,6 +350,8 @@ describe('AskUserDialog', () => { questions={multiQuestions} onSubmit={onSubmit} onCancel={vi.fn()} + width={120} + availableHeight={20} />, { width: 120 }, ); @@ -373,6 +419,8 @@ describe('AskUserDialog', () => { questions={multiQuestions} onSubmit={vi.fn()} onCancel={vi.fn()} + width={120} + availableHeight={20} />, { width: 120 }, ); @@ -401,6 +449,8 @@ describe('AskUserDialog', () => { questions={multiQuestions} onSubmit={vi.fn()} onCancel={vi.fn()} + width={120} + availableHeight={20} />, { width: 120 }, ); @@ -445,6 +495,8 @@ describe('AskUserDialog', () => { questions={multiQuestions} onSubmit={vi.fn()} onCancel={vi.fn()} + width={120} + availableHeight={20} />, { width: 120 }, ); @@ -480,6 +532,8 @@ describe('AskUserDialog', () => { questions={multiQuestions} onSubmit={onSubmit} onCancel={vi.fn()} + width={120} + availableHeight={20} />, { width: 120 }, ); @@ -512,6 +566,8 @@ describe('AskUserDialog', () => { questions={textQuestion} onSubmit={vi.fn()} onCancel={vi.fn()} + width={120} + availableHeight={20} />, { width: 120 }, ); @@ -533,6 +589,8 @@ describe('AskUserDialog', () => { questions={textQuestion} onSubmit={vi.fn()} onCancel={vi.fn()} + width={120} + availableHeight={20} />, { width: 120 }, ); @@ -554,6 +612,8 @@ describe('AskUserDialog', () => { questions={textQuestion} onSubmit={vi.fn()} onCancel={vi.fn()} + width={120} + availableHeight={20} />, { width: 120 }, ); @@ -588,6 +648,8 @@ describe('AskUserDialog', () => { questions={textQuestion} onSubmit={vi.fn()} onCancel={vi.fn()} + width={120} + availableHeight={20} />, { width: 120 }, ); @@ -618,6 +680,8 @@ describe('AskUserDialog', () => { questions={mixedQuestions} onSubmit={vi.fn()} onCancel={vi.fn()} + width={120} + availableHeight={20} />, { width: 120 }, ); @@ -664,6 +728,8 @@ describe('AskUserDialog', () => { questions={mixedQuestions} onSubmit={onSubmit} onCancel={vi.fn()} + width={120} + availableHeight={20} />, { width: 120 }, ); @@ -713,6 +779,8 @@ describe('AskUserDialog', () => { questions={textQuestion} onSubmit={onSubmit} onCancel={vi.fn()} + width={120} + availableHeight={20} />, { width: 120 }, ); @@ -738,6 +806,8 @@ describe('AskUserDialog', () => { questions={textQuestion} onSubmit={vi.fn()} onCancel={onCancel} + width={120} + availableHeight={20} />, { width: 120 }, ); @@ -783,6 +853,8 @@ describe('AskUserDialog', () => { questions={multiQuestions} onSubmit={vi.fn()} onCancel={vi.fn()} + width={120} + availableHeight={20} />, { width: 120 }, ); @@ -841,6 +913,8 @@ describe('AskUserDialog', () => { questions={multiQuestions} onSubmit={onSubmit} onCancel={vi.fn()} + width={120} + availableHeight={20} />, { width: 120 }, ); diff --git a/packages/cli/src/ui/components/AskUserDialog.tsx b/packages/cli/src/ui/components/AskUserDialog.tsx index 4c74f2fd37..e2892feade 100644 --- a/packages/cli/src/ui/components/AskUserDialog.tsx +++ b/packages/cli/src/ui/components/AskUserDialog.tsx @@ -5,14 +5,7 @@ */ import type React from 'react'; -import { - useCallback, - useMemo, - useRef, - useEffect, - useReducer, - useContext, -} from 'react'; +import { useCallback, useMemo, useRef, useEffect, useReducer } from 'react'; import { Box, Text } from 'ink'; import { theme } from '../semantic-colors.js'; import type { Question } from '@google/gemini-cli-core'; @@ -24,10 +17,10 @@ import { keyMatchers, Command } from '../keyMatchers.js'; import { checkExhaustive } from '../../utils/checks.js'; import { TextInput } from './shared/TextInput.js'; import { useTextBuffer } from './shared/text-buffer.js'; -import { UIStateContext } from '../contexts/UIStateContext.js'; import { getCachedStringWidth } from '../utils/textUtils.js'; import { useTabbedNavigation } from '../hooks/useTabbedNavigation.js'; import { DialogFooter } from './shared/DialogFooter.js'; +import { MaxSizedBox } from './shared/MaxSizedBox.js'; interface AskUserDialogState { answers: { [key: string]: string }; @@ -121,6 +114,14 @@ interface AskUserDialogProps { * Useful for managing global keypress handlers. */ onActiveTextInputChange?: (active: boolean) => void; + /** + * Width of the dialog. + */ + width: number; + /** + * Height constraint for scrollable content. + */ + availableHeight: number; } interface ReviewViewProps { @@ -152,12 +153,7 @@ const ReviewView: React.FC = ({ ); return ( - + {progressHeader} @@ -174,15 +170,19 @@ const ReviewView: React.FC = ({ )} - {questions.map((q, i) => ( - - {q.header} - → - - {answers[i] || '(not answered)'} - - - ))} + + {questions.map((q, i) => ( + + {q.header} + → + + {answers[i] || '(not answered)'} + + + ))} + void; onEditingCustomOption?: (editing: boolean) => void; availableWidth: number; + availableHeight: number; initialAnswer?: string; progressHeader?: React.ReactNode; keyboardHints?: React.ReactNode; @@ -210,12 +211,13 @@ const TextQuestionView: React.FC = ({ onSelectionChange, onEditingCustomOption, availableWidth, + availableHeight, initialAnswer, progressHeader, keyboardHints, }) => { const prefix = '> '; - const horizontalPadding = 4 + 1; // Padding from Box (2) and border (2) + 1 for cursor + const horizontalPadding = 1; // 1 for cursor const bufferWidth = availableWidth - getCachedStringWidth(prefix) - horizontalPadding; @@ -241,12 +243,15 @@ const TextQuestionView: React.FC = ({ const handleExtraKeys = useCallback( (key: Key) => { if (keyMatchers[Command.QUIT](key)) { + if (textValue === '') { + return false; + } buffer.setText(''); return true; } return false; }, - [buffer], + [buffer, textValue], ); useKeypress(handleExtraKeys, { isActive: true, priority: true }); @@ -270,18 +275,21 @@ const TextQuestionView: React.FC = ({ const placeholder = question.placeholder || 'Enter your response'; + const HEADER_HEIGHT = progressHeader ? 2 : 0; + const INPUT_HEIGHT = 2; // TextInput + margin + const FOOTER_HEIGHT = 2; // DialogFooter + margin + const overhead = HEADER_HEIGHT + INPUT_HEIGHT + FOOTER_HEIGHT; + const questionHeight = Math.max(1, availableHeight - overhead); + return ( - + {progressHeader} - - {question.question} - + + + {question.question} + + @@ -381,6 +389,7 @@ interface ChoiceQuestionViewProps { onSelectionChange?: (answer: string) => void; onEditingCustomOption?: (editing: boolean) => void; availableWidth: number; + availableHeight: number; initialAnswer?: string; progressHeader?: React.ReactNode; keyboardHints?: React.ReactNode; @@ -391,14 +400,12 @@ const ChoiceQuestionView: React.FC = ({ onAnswer, onSelectionChange, onEditingCustomOption, + availableWidth, + availableHeight, initialAnswer, progressHeader, keyboardHints, }) => { - const uiState = useContext(UIStateContext); - const terminalWidth = uiState?.terminalWidth ?? 80; - const availableWidth = terminalWidth; - const numOptions = (question.options?.length ?? 0) + (question.type !== 'yesno' ? 1 : 0); const numLen = String(numOptions).length; @@ -407,15 +414,9 @@ const ChoiceQuestionView: React.FC = ({ const checkboxWidth = question.multiSelect ? 4 : 1; // "[x] " or " " const checkmarkWidth = question.multiSelect ? 0 : 2; // "" or " ✓" const cursorPadding = 1; // Extra character for cursor at end of line - const outerBoxPadding = 4; // border (2) + paddingX (2) const horizontalPadding = - outerBoxPadding + - radioWidth + - numberWidth + - checkboxWidth + - checkmarkWidth + - cursorPadding; + radioWidth + numberWidth + checkboxWidth + checkmarkWidth + cursorPadding; const bufferWidth = availableWidth - horizontalPadding; @@ -544,6 +545,9 @@ const ChoiceQuestionView: React.FC = ({ (key: Key) => { // If focusing custom option, handle Ctrl+C if (isCustomOptionFocused && keyMatchers[Command.QUIT](key)) { + if (customOptionText === '') { + return false; + } customBuffer.setText(''); return true; } @@ -586,7 +590,12 @@ const ChoiceQuestionView: React.FC = ({ } return false; }, - [isCustomOptionFocused, customBuffer, onEditingCustomOption], + [ + isCustomOptionFocused, + customBuffer, + onEditingCustomOption, + customOptionText, + ], ); useKeypress(handleExtraKeys, { isActive: true, priority: true }); @@ -698,31 +707,41 @@ const ChoiceQuestionView: React.FC = ({ } }, [customOptionText, isCustomOptionSelected, question.multiSelect]); + const HEADER_HEIGHT = progressHeader ? 2 : 0; + const TITLE_MARGIN = 1; + const FOOTER_HEIGHT = 2; // DialogFooter + margin + const overhead = HEADER_HEIGHT + TITLE_MARGIN + FOOTER_HEIGHT; + const listHeight = Math.max(1, availableHeight - overhead); + const questionHeight = Math.min(3, Math.max(1, listHeight - 4)); + const maxItemsToShow = Math.max( + 1, + Math.floor((listHeight - questionHeight) / 2), + ); + return ( - + {progressHeader} - - - {question.question} - + + + + {question.question} + {question.multiSelect && ( + + {' '} + (Select all that apply) + + )} + + - {question.multiSelect && ( - - {' '} - (Select all that apply) - - )} items={selectionItems} onSelect={handleSelect} onHighlight={handleHighlight} focusKey={isCustomOptionFocused ? 'other' : undefined} + maxItemsToShow={maxItemsToShow} + showScrollArrows={true} renderItem={(item, context) => { const optionItem = item.value; const isChecked = @@ -804,14 +823,12 @@ export const AskUserDialog: React.FC = ({ onSubmit, onCancel, onActiveTextInputChange, + width, + availableHeight, }) => { const [state, dispatch] = useReducer(askUserDialogReducerLogic, initialState); const { answers, isEditingCustomOption, submitted } = state; - const uiState = useContext(UIStateContext); - const terminalWidth = uiState?.terminalWidth ?? 80; - const availableWidth = terminalWidth; - const reviewTabIndex = questions.length; const tabCount = questions.length > 1 ? questions.length + 1 : questions.length; @@ -842,9 +859,12 @@ export const AskUserDialog: React.FC = ({ if (keyMatchers[Command.ESCAPE](key)) { onCancel(); return true; - } else if (keyMatchers[Command.QUIT](key) && !isEditingCustomOption) { - onCancel(); - return true; + } else if (keyMatchers[Command.QUIT](key)) { + if (!isEditingCustomOption) { + onCancel(); + } + // Return false to let ctrl-C bubble up to AppContainer for exit flow + return false; } return false; }, @@ -1021,7 +1041,8 @@ export const AskUserDialog: React.FC = ({ onAnswer={handleAnswer} onSelectionChange={handleSelectionChange} onEditingCustomOption={handleEditingCustomOption} - availableWidth={availableWidth} + availableWidth={width} + availableHeight={availableHeight} initialAnswer={answers[currentQuestionIndex]} progressHeader={progressHeader} keyboardHints={keyboardHints} @@ -1033,7 +1054,8 @@ export const AskUserDialog: React.FC = ({ onAnswer={handleAnswer} onSelectionChange={handleSelectionChange} onEditingCustomOption={handleEditingCustomOption} - availableWidth={availableWidth} + availableWidth={width} + availableHeight={availableHeight} initialAnswer={answers[currentQuestionIndex]} progressHeader={progressHeader} keyboardHints={keyboardHints} @@ -1043,7 +1065,7 @@ export const AskUserDialog: React.FC = ({ return ( {questionView} diff --git a/packages/cli/src/ui/components/BubblingRegression.test.tsx b/packages/cli/src/ui/components/BubblingRegression.test.tsx index a7a0e31714..f91f6fe2dc 100644 --- a/packages/cli/src/ui/components/BubblingRegression.test.tsx +++ b/packages/cli/src/ui/components/BubblingRegression.test.tsx @@ -34,6 +34,8 @@ describe('Key Bubbling Regression', () => { questions={choiceQuestion} onSubmit={vi.fn()} onCancel={vi.fn()} + width={120} + availableHeight={20} />, { width: 120 }, ); diff --git a/packages/cli/src/ui/components/DialogManager.tsx b/packages/cli/src/ui/components/DialogManager.tsx index 527c2de983..6d4db7ca3b 100644 --- a/packages/cli/src/ui/components/DialogManager.tsx +++ b/packages/cli/src/ui/components/DialogManager.tsx @@ -32,8 +32,6 @@ import process from 'node:process'; import { type UseHistoryManagerReturn } from '../hooks/useHistoryManager.js'; import { AdminSettingsChangedDialog } from './AdminSettingsChangedDialog.js'; import { IdeTrustChangeDialog } from './IdeTrustChangeDialog.js'; -import { AskUserDialog } from './AskUserDialog.js'; -import { useAskUserActions } from '../contexts/AskUserActionsContext.js'; import { NewAgentsNotification } from './NewAgentsNotification.js'; import { AgentConfigDialog } from './AgentConfigDialog.js'; @@ -59,22 +57,6 @@ export const DialogManager = ({ terminalWidth: uiTerminalWidth, } = uiState; - const { - request: askUserRequest, - submit: askUserSubmit, - cancel: askUserCancel, - } = useAskUserActions(); - - if (askUserRequest) { - return ( - - ); - } - if (uiState.adminSettingsChanged) { return ; } diff --git a/packages/cli/src/ui/components/ToolConfirmationQueue.tsx b/packages/cli/src/ui/components/ToolConfirmationQueue.tsx index 71f57b7a3a..0ee6fec05c 100644 --- a/packages/cli/src/ui/components/ToolConfirmationQueue.tsx +++ b/packages/cli/src/ui/components/ToolConfirmationQueue.tsx @@ -16,6 +16,21 @@ import { OverflowProvider } from '../contexts/OverflowContext.js'; import { ShowMoreLines } from './ShowMoreLines.js'; import { StickyHeader } from './StickyHeader.js'; import { useAlternateBuffer } from '../hooks/useAlternateBuffer.js'; +import type { SerializableConfirmationDetails } from '@google/gemini-cli-core'; + +function getConfirmationHeader( + details: SerializableConfirmationDetails | undefined, +): string { + const headers: Partial< + Record + > = { + ask_user: 'Answer Questions', + }; + if (!details?.type) { + return 'Action Required'; + } + return headers[details.type] ?? 'Action Required'; +} interface ToolConfirmationQueueProps { confirmingTool: ConfirmingToolState; @@ -55,6 +70,7 @@ export const ToolConfirmationQueue: React.FC = ({ : undefined; const borderColor = theme.status.warning; + const hideToolIdentity = tool.confirmationDetails?.type === 'ask_user'; return ( @@ -67,25 +83,31 @@ export const ToolConfirmationQueue: React.FC = ({ > {/* Header */} - + - Action Required - - - {index} of {total} + {getConfirmationHeader(tool.confirmationDetails)} + {total > 1 && ( + + {index} of {total} + + )} - {/* Tool Identity (Context) */} - - - - + {!hideToolIdentity && ( + + + + + )} diff --git a/packages/cli/src/ui/components/__snapshots__/AskUserDialog.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/AskUserDialog.test.tsx.snap index 54554740aa..fdb34f4adb 100644 --- a/packages/cli/src/ui/components/__snapshots__/AskUserDialog.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/AskUserDialog.test.tsx.snap @@ -1,138 +1,131 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html exports[`AskUserDialog > Text type questions > renders text input for type: "text" 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮ -│ What should we name this component? │ -│ │ -│ > e.g., UserProfileCard │ -│ │ -│ │ -│ Enter to submit · Esc to cancel │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯" +"What should we name this component? + +> e.g., UserProfileCard + + +Enter to submit · Esc to cancel" `; exports[`AskUserDialog > Text type questions > shows correct keyboard hints for text type 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮ -│ Enter the variable name: │ -│ │ -│ > Enter your response │ -│ │ -│ │ -│ Enter to submit · Esc to cancel │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯" +"Enter the variable name: + +> Enter your response + + +Enter to submit · Esc to cancel" `; exports[`AskUserDialog > Text type questions > shows default placeholder when none provided 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮ -│ Enter the database connection string: │ -│ │ -│ > Enter your response │ -│ │ -│ │ -│ Enter to submit · Esc to cancel │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯" +"Enter the database connection string: + +> Enter your response + + +Enter to submit · Esc to cancel" `; exports[`AskUserDialog > allows navigating to Review tab and back 1`] = ` -"╭─────────────────────────────────────────────────────────────────╮ -│ ← □ Tests │ □ Docs │ ≡ Review → │ -│ │ -│ Review your answers: │ -│ │ -│ ⚠ You have 2 unanswered questions │ -│ │ -│ Tests → (not answered) │ -│ Docs → (not answered) │ -│ │ -│ Enter to submit · Tab/Shift+Tab to edit answers · Esc to cancel │ -╰─────────────────────────────────────────────────────────────────╯" +"← □ Tests │ □ Docs │ ≡ Review → + +Review your answers: + +⚠ You have 2 unanswered questions + +Tests → (not answered) +Docs → (not answered) + +Enter to submit · Tab/Shift+Tab to edit answers · Esc to cancel" `; exports[`AskUserDialog > hides progress header for single question 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮ -│ Which authentication method should we use? │ -│ │ -│ ● 1. OAuth 2.0 │ -│ Industry standard, supports SSO │ -│ 2. JWT tokens │ -│ Stateless, good for APIs │ -│ 3. Enter a custom value │ -│ │ -│ Enter to select · ↑/↓ to navigate · Esc to cancel │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯" +"Which authentication method should we use? + +● 1. OAuth 2.0 + Industry standard, supports SSO + 2. JWT tokens + Stateless, good for APIs + 3. Enter a custom value + +Enter to select · ↑/↓ to navigate · Esc to cancel" `; exports[`AskUserDialog > renders question and options 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮ -│ Which authentication method should we use? │ -│ │ -│ ● 1. OAuth 2.0 │ -│ Industry standard, supports SSO │ -│ 2. JWT tokens │ -│ Stateless, good for APIs │ -│ 3. Enter a custom value │ -│ │ -│ Enter to select · ↑/↓ to navigate · Esc to cancel │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯" +"Which authentication method should we use? + +● 1. OAuth 2.0 + Industry standard, supports SSO + 2. JWT tokens + Stateless, good for APIs + 3. Enter a custom value + +Enter to select · ↑/↓ to navigate · Esc to cancel" `; exports[`AskUserDialog > shows Review tab in progress header for multiple questions 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮ -│ ← □ Framework │ □ Styling │ ≡ Review → │ -│ │ -│ Which framework? │ -│ │ -│ ● 1. React │ -│ Component library │ -│ 2. Vue │ -│ Progressive framework │ -│ 3. Enter a custom value │ -│ │ -│ Enter to select · ←/→ to switch questions · Esc to cancel │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯" +"← □ Framework │ □ Styling │ ≡ Review → + +Which framework? + +● 1. React + Component library + 2. Vue + Progressive framework + 3. Enter a custom value + +Enter to select · ←/→ to switch questions · Esc to cancel" `; exports[`AskUserDialog > shows keyboard hints 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮ -│ Which authentication method should we use? │ -│ │ -│ ● 1. OAuth 2.0 │ -│ Industry standard, supports SSO │ -│ 2. JWT tokens │ -│ Stateless, good for APIs │ -│ 3. Enter a custom value │ -│ │ -│ Enter to select · ↑/↓ to navigate · Esc to cancel │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯" +"Which authentication method should we use? + +● 1. OAuth 2.0 + Industry standard, supports SSO + 2. JWT tokens + Stateless, good for APIs + 3. Enter a custom value + +Enter to select · ↑/↓ to navigate · Esc to cancel" `; exports[`AskUserDialog > shows progress header for multiple questions 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮ -│ ← □ Database │ □ ORM │ ≡ Review → │ -│ │ -│ Which database should we use? │ -│ │ -│ ● 1. PostgreSQL │ -│ Relational database │ -│ 2. MongoDB │ -│ Document database │ -│ 3. Enter a custom value │ -│ │ -│ Enter to select · ←/→ to switch questions · Esc to cancel │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯" +"← □ Database │ □ ORM │ ≡ Review → + +Which database should we use? + +● 1. PostgreSQL + Relational database + 2. MongoDB + Document database + 3. Enter a custom value + +Enter to select · ←/→ to switch questions · Esc to cancel" +`; + +exports[`AskUserDialog > shows scroll arrows when options exceed available height 1`] = ` +"Choose an option + +▲ +● 1. Option 1 + Description 1 + 2. Option 2 + Description 2 +▼ + +Enter to select · ↑/↓ to navigate · Esc to cancel" `; exports[`AskUserDialog > shows warning for unanswered questions on Review tab 1`] = ` -"╭─────────────────────────────────────────────────────────────────╮ -│ ← □ License │ □ README │ ≡ Review → │ -│ │ -│ Review your answers: │ -│ │ -│ ⚠ You have 2 unanswered questions │ -│ │ -│ License → (not answered) │ -│ README → (not answered) │ -│ │ -│ Enter to submit · Tab/Shift+Tab to edit answers · Esc to cancel │ -╰─────────────────────────────────────────────────────────────────╯" +"← □ License │ □ README │ ≡ Review → + +Review your answers: + +⚠ You have 2 unanswered questions + +License → (not answered) +README → (not answered) + +Enter to submit · Tab/Shift+Tab to edit answers · Esc to cancel" `; diff --git a/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue.test.tsx.snap index 8f5abfa7a9..a4238e2028 100644 --- a/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue.test.tsx.snap @@ -2,7 +2,7 @@ exports[`ToolConfirmationQueue > calculates availableContentHeight based on availableTerminalHeight from UI state 1`] = ` "╭──────────────────────────────────────────────────────────────────────────────╮ -│ Action Required 1 of 1 │ +│ Action Required │ │ │ │ ? replace edit file │ │ │ @@ -22,7 +22,7 @@ exports[`ToolConfirmationQueue > calculates availableContentHeight based on avai exports[`ToolConfirmationQueue > does not render expansion hint when constrainHeight is false 1`] = ` "╭──────────────────────────────────────────────────────────────────────────────╮ -│ Action Required 1 of 1 │ +│ Action Required │ │ │ │ ? replace edit file │ │ │ @@ -44,7 +44,7 @@ exports[`ToolConfirmationQueue > does not render expansion hint when constrainHe exports[`ToolConfirmationQueue > renders expansion hint when content is long and constrained 1`] = ` "╭──────────────────────────────────────────────────────────────────────────────╮ -│ Action Required 1 of 1 │ +│ Action Required │ │ │ │ ? replace edit file │ │ │ diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx index e200f0bb44..e58d0ca600 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx @@ -25,12 +25,14 @@ import { sanitizeForDisplay } from '../../utils/textUtils.js'; import { useKeypress } from '../../hooks/useKeypress.js'; import { theme } from '../../semantic-colors.js'; import { useSettings } from '../../contexts/SettingsContext.js'; +import { keyMatchers, Command } from '../../keyMatchers.js'; import { REDIRECTION_WARNING_NOTE_LABEL, REDIRECTION_WARNING_NOTE_TEXT, REDIRECTION_WARNING_TIP_LABEL, REDIRECTION_WARNING_TIP_TEXT, } from '../../textConstants.js'; +import { AskUserDialog } from '../AskUserDialog.js'; export interface ToolConfirmationMessageProps { callId: string; @@ -59,9 +61,15 @@ export const ToolConfirmationMessage: React.FC< const allowPermanentApproval = settings.merged.security.enablePermanentToolApproval; + const handlesOwnUI = confirmationDetails.type === 'ask_user'; + const isTrustedFolder = config.isTrustedFolder(); + const handleConfirm = useCallback( - (outcome: ToolConfirmationOutcome) => { - void confirm(callId, outcome).catch((error: unknown) => { + ( + outcome: ToolConfirmationOutcome, + payload?: { answers?: { [questionIndex: string]: string } }, + ) => { + void confirm(callId, outcome, payload).catch((error: unknown) => { debugLogger.error( `Failed to handle tool confirmation for ${callId}:`, error, @@ -71,15 +79,18 @@ export const ToolConfirmationMessage: React.FC< [confirm, callId], ); - const isTrustedFolder = config.isTrustedFolder(); - useKeypress( (key) => { if (!isFocused) return false; - if (key.name === 'escape' || (key.ctrl && key.name === 'c')) { + if (keyMatchers[Command.ESCAPE](key)) { handleConfirm(ToolConfirmationOutcome.Cancel); return true; } + if (keyMatchers[Command.QUIT](key)) { + // Return false to let ctrl-C bubble up to AppContainer for exit flow. + // AppContainer will call cancelOngoingRequest which will cancel the tool. + return false; + } return false; }, { isActive: isFocused }, @@ -180,7 +191,7 @@ export const ToolConfirmationMessage: React.FC< value: ToolConfirmationOutcome.Cancel, key: 'No, suggest changes (esc)', }); - } else { + } else if (confirmationDetails.type === 'mcp') { // mcp tool confirmation options.push({ label: 'Allow once', @@ -251,6 +262,23 @@ export const ToolConfirmationMessage: React.FC< let question = ''; const options = getOptions(); + if (confirmationDetails.type === 'ask_user') { + bodyContent = ( + { + handleConfirm(ToolConfirmationOutcome.ProceedOnce, { answers }); + }} + onCancel={() => { + handleConfirm(ToolConfirmationOutcome.Cancel); + }} + width={terminalWidth} + availableHeight={availableBodyContentHeight() ?? 10} + /> + ); + return { question: '', bodyContent, options: [] }; + } + if (confirmationDetails.type === 'edit') { if (!confirmationDetails.isModifying) { question = `Apply this change?`; @@ -265,7 +293,7 @@ export const ToolConfirmationMessage: React.FC< } } else if (confirmationDetails.type === 'info') { question = `Do you want to proceed?`; - } else { + } else if (confirmationDetails.type === 'mcp') { // mcp tool confirmation const mcpProps = confirmationDetails; question = `Allow execution of MCP tool "${mcpProps.toolName}" from server "${mcpProps.serverName}"?`; @@ -387,7 +415,7 @@ export const ToolConfirmationMessage: React.FC< )} ); - } else { + } else if (confirmationDetails.type === 'mcp') { // mcp tool confirmation const mcpProps = confirmationDetails; @@ -405,6 +433,7 @@ export const ToolConfirmationMessage: React.FC< getOptions, availableBodyContentHeight, terminalWidth, + handleConfirm, ]); if (confirmationDetails.type === 'edit') { @@ -429,32 +458,38 @@ export const ToolConfirmationMessage: React.FC< } return ( - - {/* Body Content (Diff Renderer or Command Info) */} - {/* No separate context display here anymore for edits */} - - - {bodyContent} - - + + {handlesOwnUI ? ( + bodyContent + ) : ( + <> + + + {bodyContent} + + - {/* Confirmation Question */} - - {question} - + + {question} + - {/* Select Input for Options */} - - - + + + + + )} ); }; diff --git a/packages/cli/src/ui/components/shared/BaseSelectionList.test.tsx b/packages/cli/src/ui/components/shared/BaseSelectionList.test.tsx index 0d1eb43f6c..c671399baf 100644 --- a/packages/cli/src/ui/components/shared/BaseSelectionList.test.tsx +++ b/packages/cli/src/ui/components/shared/BaseSelectionList.test.tsx @@ -475,14 +475,7 @@ describe('BaseSelectionList', () => { ); await waitFor(() => { - const output = lastFrame(); - // At the top, should show first 3 items - expect(output).toContain('Item 1'); - expect(output).toContain('Item 3'); - expect(output).not.toContain('Item 4'); - // Both arrows should be visible - expect(output).toContain('▲'); - expect(output).toContain('▼'); + expect(lastFrame()).toMatchSnapshot(); }); }); @@ -493,15 +486,7 @@ describe('BaseSelectionList', () => { ); await waitFor(() => { - const output = lastFrame(); - // After scrolling to middle, should see items around index 5 - expect(output).toContain('Item 4'); - expect(output).toContain('Item 6'); - expect(output).not.toContain('Item 3'); - expect(output).not.toContain('Item 7'); - // Both scroll arrows should be visible - expect(output).toContain('▲'); - expect(output).toContain('▼'); + expect(lastFrame()).toMatchSnapshot(); }); }); @@ -512,32 +497,18 @@ describe('BaseSelectionList', () => { ); await waitFor(() => { - const output = lastFrame(); - // At the end, should show last 3 items - expect(output).toContain('Item 8'); - expect(output).toContain('Item 10'); - expect(output).not.toContain('Item 7'); - // Both arrows should be visible - expect(output).toContain('▲'); - expect(output).toContain('▼'); + expect(lastFrame()).toMatchSnapshot(); }); }); - it('should show both arrows dimmed when list fits entirely', () => { + it('should not show arrows when list fits entirely', () => { const { lastFrame } = renderComponent({ items, maxItemsToShow: 5, showScrollArrows: true, }); - const output = lastFrame(); - // Should show all items since maxItemsToShow > items.length - expect(output).toContain('Item A'); - expect(output).toContain('Item B'); - expect(output).toContain('Item C'); - // Both arrows should be visible but dimmed (this test doesn't need waitFor since no scrolling occurs) - expect(output).toContain('▲'); - expect(output).toContain('▼'); + expect(lastFrame()).toMatchSnapshot(); }); }); }); diff --git a/packages/cli/src/ui/components/shared/BaseSelectionList.tsx b/packages/cli/src/ui/components/shared/BaseSelectionList.tsx index baec1bb8ca..db0d624a74 100644 --- a/packages/cli/src/ui/components/shared/BaseSelectionList.tsx +++ b/packages/cli/src/ui/components/shared/BaseSelectionList.tsx @@ -100,7 +100,7 @@ export function BaseSelectionList< return ( {/* Use conditional coloring instead of conditional rendering */} - {showScrollArrows && ( + {showScrollArrows && items.length > maxItemsToShow && ( 0 ? theme.text.primary : theme.text.secondary} > @@ -172,7 +172,7 @@ export function BaseSelectionList< ); })} - {showScrollArrows && ( + {showScrollArrows && items.length > maxItemsToShow && ( Scroll Arrows (showScrollArrows) > should not show arrows when list fits entirely 1`] = ` +"● 1. Item A + 2. Item B + 3. Item C" +`; + +exports[`BaseSelectionList > Scroll Arrows (showScrollArrows) > should show arrows and correct items when scrolled to the end 1`] = ` +"▲ + 8. Item 8 + 9. Item 9 +● 10. Item 10 +▼" +`; + +exports[`BaseSelectionList > Scroll Arrows (showScrollArrows) > should show arrows and correct items when scrolled to the middle 1`] = ` +"▲ + 4. Item 4 + 5. Item 5 +● 6. Item 6 +▼" +`; + +exports[`BaseSelectionList > Scroll Arrows (showScrollArrows) > should show arrows with correct colors when enabled (at the top) 1`] = ` +"▲ +● 1. Item 1 + 2. Item 2 + 3. Item 3 +▼" +`; diff --git a/packages/cli/src/ui/components/shared/__snapshots__/DescriptiveRadioButtonSelect.test.tsx.snap b/packages/cli/src/ui/components/shared/__snapshots__/DescriptiveRadioButtonSelect.test.tsx.snap index 822b88b0c8..da306c2823 100644 --- a/packages/cli/src/ui/components/shared/__snapshots__/DescriptiveRadioButtonSelect.test.tsx.snap +++ b/packages/cli/src/ui/components/shared/__snapshots__/DescriptiveRadioButtonSelect.test.tsx.snap @@ -1,14 +1,12 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html exports[`DescriptiveRadioButtonSelect > should render correctly with custom props 1`] = ` -"▲ - 1. Foo Title +" 1. Foo Title This is Foo. ● 2. Bar Title This is Bar. 3. Baz Title - This is Baz. -▼" + This is Baz." `; exports[`DescriptiveRadioButtonSelect > should render correctly with default props 1`] = ` diff --git a/packages/cli/src/ui/utils/__snapshots__/TableRenderer.test.tsx.snap b/packages/cli/src/ui/utils/__snapshots__/TableRenderer.test.tsx.snap index 8f3380f51b..0f9e0b84d5 100644 --- a/packages/cli/src/ui/utils/__snapshots__/TableRenderer.test.tsx.snap +++ b/packages/cli/src/ui/utils/__snapshots__/TableRenderer.test.tsx.snap @@ -1,35 +1,5 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html -exports[`TableRenderer > handles empty rows 1`] = ` -" -┌──────┬──────┬────────┐ -│ Name │ Role │ Status │ -├──────┼──────┼────────┤ -└──────┴──────┴────────┘ -" -`; - -exports[`TableRenderer > handles markdown content in cells 1`] = ` -" -┌───────┬──────────┬────────┐ -│ Name │ Role │ Status │ -├───────┼──────────┼────────┤ -│ Alice │ Engineer │ Active │ -└───────┴──────────┴────────┘ -" -`; - -exports[`TableRenderer > handles rows with missing cells 1`] = ` -" -┌───────┬──────────┬────────┐ -│ Name │ Role │ Status │ -├───────┼──────────┼────────┤ -│ Alice │ Engineer │ -│ Bob │ -└───────┴──────────┴────────┘ -" -`; - exports[`TableRenderer > renders a 3x3 table correctly 1`] = ` " ┌──────────────┬──────────────┬──────────────┐ @@ -42,18 +12,6 @@ exports[`TableRenderer > renders a 3x3 table correctly 1`] = ` " `; -exports[`TableRenderer > renders a simple table correctly 1`] = ` -" -┌─────────┬──────────┬──────────┐ -│ Name │ Role │ Status │ -├─────────┼──────────┼──────────┤ -│ Alice │ Engineer │ Active │ -│ Bob │ Designer │ Inactive │ -│ Charlie │ Manager │ Active │ -└─────────┴──────────┴──────────┘ -" -`; - exports[`TableRenderer > renders a table with long headers and 4 columns correctly 1`] = ` " ┌──────────────────┬──────────────────┬───────────────────┬──────────────────┐ @@ -65,25 +23,3 @@ exports[`TableRenderer > renders a table with long headers and 4 columns correct └──────────────────┴──────────────────┴───────────────────┴──────────────────┘ " `; - -exports[`TableRenderer > truncates content when terminal width is small 1`] = ` -" -┌────────┬─────────┬─────────┐ -│ Name │ Role │ Status │ -├────────┼─────────┼─────────┤ -│ Alice │ Engi... │ Active │ -│ Bob │ Desi... │ Inac... │ -│ Cha... │ Manager │ Active │ -└────────┴─────────┴─────────┘ -" -`; - -exports[`TableRenderer > truncates long markdown content correctly 1`] = ` -" -┌───────────────────────────┬─────┬────┐ -│ Name │ Rol │ St │ -├───────────────────────────┼─────┼────┤ -│ Alice with a very long... │ Eng │ Ac │ -└───────────────────────────┴─────┴────┘ -" -`; diff --git a/packages/cli/src/zed-integration/zedIntegration.ts b/packages/cli/src/zed-integration/zedIntegration.ts index 1769cbafca..7273c0b961 100644 --- a/packages/cli/src/zed-integration/zedIntegration.ts +++ b/packages/cli/src/zed-integration/zedIntegration.ts @@ -983,6 +983,9 @@ function toPermissionOptions( }, ...basicPermissionOptions, ]; + case 'ask_user': + // askuser doesn't need "always allow" options since it's asking questions + return [...basicPermissionOptions]; default: { const unreachable: never = confirmation; throw new Error(`Unexpected: ${unreachable}`); diff --git a/packages/core/src/confirmation-bus/types.ts b/packages/core/src/confirmation-bus/types.ts index 5a27b08d40..fcdd600f3c 100644 --- a/packages/core/src/confirmation-bus/types.ts +++ b/packages/core/src/confirmation-bus/types.ts @@ -65,7 +65,12 @@ export interface ToolConfirmationResponse { * Data-only versions of ToolCallConfirmationDetails for bus transmission. */ export type SerializableConfirmationDetails = - | { type: 'info'; title: string; prompt: string; urls?: string[] } + | { + type: 'info'; + title: string; + prompt: string; + urls?: string[]; + } | { type: 'edit'; title: string; @@ -90,6 +95,11 @@ export type SerializableConfirmationDetails = serverName: string; toolName: string; toolDisplayName: string; + } + | { + type: 'ask_user'; + title: string; + questions: Question[]; }; export interface UpdatePolicy { diff --git a/packages/core/src/policy/policies/plan.toml b/packages/core/src/policy/policies/plan.toml index 5e8f51f793..5b8b1d7882 100644 --- a/packages/core/src/policy/policies/plan.toml +++ b/packages/core/src/policy/policies/plan.toml @@ -66,7 +66,7 @@ modes = ["plan"] [[rule]] toolName = "ask_user" -decision = "allow" +decision = "ask_user" priority = 50 modes = ["plan"] diff --git a/packages/core/src/scheduler/confirmation.ts b/packages/core/src/scheduler/confirmation.ts index 73958815d0..76ab6fac97 100644 --- a/packages/core/src/scheduler/confirmation.ts +++ b/packages/core/src/scheduler/confirmation.ts @@ -150,6 +150,10 @@ export async function resolveConfirmation( ); outcome = response.outcome; + if ('onConfirm' in details && typeof details.onConfirm === 'function') { + await details.onConfirm(outcome, response.payload); + } + if (outcome === ToolConfirmationOutcome.ModifyWithEditor) { await handleExternalModification(deps, toolCall, signal); } else if (response.payload?.newContent) { diff --git a/packages/core/src/tools/ask-user.test.ts b/packages/core/src/tools/ask-user.test.ts index 01dfefb2ee..da41ff45f2 100644 --- a/packages/core/src/tools/ask-user.test.ts +++ b/packages/core/src/tools/ask-user.test.ts @@ -6,12 +6,9 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { AskUserTool } from './ask-user.js'; -import { - MessageBusType, - QuestionType, - type Question, -} from '../confirmation-bus/types.js'; +import { QuestionType, type Question } from '../confirmation-bus/types.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; +import { ToolConfirmationOutcome } from './tools.js'; describe('AskUserTool', () => { let mockMessageBus: MessageBus; @@ -213,142 +210,183 @@ describe('AskUserTool', () => { }); }); - it('should publish ASK_USER_REQUEST and wait for response', async () => { - const questions = [ - { - question: 'How should we proceed with this task?', - header: 'Approach', - options: [ - { - label: 'Quick fix (Recommended)', - description: - 'Apply the most direct solution to resolve the immediate issue.', - }, - { - label: 'Comprehensive refactor', - description: - 'Restructure the affected code for better long-term maintainability.', - }, - ], - multiSelect: false, - }, - ]; - - const invocation = tool.build({ questions }); - const executePromise = invocation.execute(new AbortController().signal); - - // Verify publish called with normalized questions (type defaults to CHOICE) - expect(mockMessageBus.publish).toHaveBeenCalledWith( - expect.objectContaining({ - type: MessageBusType.ASK_USER_REQUEST, - questions: questions.map((q) => ({ - ...q, - type: QuestionType.CHOICE, - })), - }), - ); - - // Get the correlation ID from the published message - const publishCall = vi.mocked(mockMessageBus.publish).mock.calls[0][0] as { - correlationId: string; - }; - const correlationId = publishCall.correlationId; - expect(correlationId).toBeDefined(); - - // Verify subscribe called - expect(mockMessageBus.subscribe).toHaveBeenCalledWith( - MessageBusType.ASK_USER_RESPONSE, - expect.any(Function), - ); - - // Simulate response - const subscribeCall = vi - .mocked(mockMessageBus.subscribe) - .mock.calls.find((call) => call[0] === MessageBusType.ASK_USER_RESPONSE); - const handler = subscribeCall![1]; - - const answers = { '0': 'Quick fix (Recommended)' }; - handler({ - type: MessageBusType.ASK_USER_RESPONSE, - correlationId, - answers, - }); - - const result = await executePromise; - expect(result.returnDisplay).toContain('User answered:'); - expect(result.returnDisplay).toContain( - ' Approach → Quick fix (Recommended)', - ); - expect(JSON.parse(result.llmContent as string)).toEqual({ answers }); - }); - - it('should display message when user submits without answering', async () => { - const questions = [ - { - question: 'Which approach?', - header: 'Approach', - options: [ - { label: 'Option A', description: 'First option' }, - { label: 'Option B', description: 'Second option' }, - ], - }, - ]; - - const invocation = tool.build({ questions }); - const executePromise = invocation.execute(new AbortController().signal); - - // Get the correlation ID from the published message - const publishCall = vi.mocked(mockMessageBus.publish).mock.calls[0][0] as { - correlationId: string; - }; - const correlationId = publishCall.correlationId; - - // Simulate response with empty answers - const subscribeCall = vi - .mocked(mockMessageBus.subscribe) - .mock.calls.find((call) => call[0] === MessageBusType.ASK_USER_RESPONSE); - const handler = subscribeCall![1]; - - handler({ - type: MessageBusType.ASK_USER_RESPONSE, - correlationId, - answers: {}, - }); - - const result = await executePromise; - expect(result.returnDisplay).toBe( - 'User submitted without answering questions.', - ); - expect(JSON.parse(result.llmContent as string)).toEqual({ answers: {} }); - }); - - it('should handle cancellation', async () => { - const invocation = tool.build({ - questions: [ + describe('shouldConfirmExecute', () => { + it('should return confirmation details with normalized questions', async () => { + const questions = [ { - question: 'Which sections of the documentation should be updated?', - header: 'Docs', + question: 'How should we proceed with this task?', + header: 'Approach', options: [ { - label: 'User Guide', - description: 'Update the main user-facing documentation.', + label: 'Quick fix (Recommended)', + description: + 'Apply the most direct solution to resolve the immediate issue.', }, { - label: 'API Reference', - description: 'Update the detailed API documentation.', + label: 'Comprehensive refactor', + description: + 'Restructure the affected code for better long-term maintainability.', }, ], - multiSelect: true, + multiSelect: false, }, - ], + ]; + + const invocation = tool.build({ questions }); + const details = await invocation.shouldConfirmExecute( + new AbortController().signal, + ); + + expect(details).not.toBe(false); + if (details && details.type === 'ask_user') { + expect(details.title).toBe('Ask User'); + expect(details.questions).toEqual( + questions.map((q) => ({ + ...q, + type: QuestionType.CHOICE, + })), + ); + expect(typeof details.onConfirm).toBe('function'); + } else { + // Type guard for TypeScript + expect(details).toBeTruthy(); + } }); - const controller = new AbortController(); - const executePromise = invocation.execute(controller.signal); + it('should normalize question type to CHOICE when omitted', async () => { + const questions = [ + { + question: 'Which approach?', + header: 'Approach', + options: [ + { label: 'Option A', description: 'First option' }, + { label: 'Option B', description: 'Second option' }, + ], + }, + ]; - controller.abort(); + const invocation = tool.build({ questions }); + const details = await invocation.shouldConfirmExecute( + new AbortController().signal, + ); - const result = await executePromise; - expect(result.error?.message).toBe('Cancelled'); + if (details && details.type === 'ask_user') { + expect(details.questions[0].type).toBe(QuestionType.CHOICE); + } + }); + }); + + describe('execute', () => { + it('should return user answers after confirmation', async () => { + const questions = [ + { + question: 'How should we proceed with this task?', + header: 'Approach', + options: [ + { + label: 'Quick fix (Recommended)', + description: + 'Apply the most direct solution to resolve the immediate issue.', + }, + { + label: 'Comprehensive refactor', + description: + 'Restructure the affected code for better long-term maintainability.', + }, + ], + multiSelect: false, + }, + ]; + + const invocation = tool.build({ questions }); + const details = await invocation.shouldConfirmExecute( + new AbortController().signal, + ); + + // Simulate confirmation with answers + if (details && 'onConfirm' in details) { + const answers = { '0': 'Quick fix (Recommended)' }; + await details.onConfirm(ToolConfirmationOutcome.ProceedOnce, { + answers, + }); + } + + const result = await invocation.execute(new AbortController().signal); + expect(result.returnDisplay).toContain('User answered:'); + expect(result.returnDisplay).toContain( + ' Approach → Quick fix (Recommended)', + ); + expect(JSON.parse(result.llmContent as string)).toEqual({ + answers: { '0': 'Quick fix (Recommended)' }, + }); + }); + + it('should display message when user submits without answering', async () => { + const questions = [ + { + question: 'Which approach?', + header: 'Approach', + options: [ + { label: 'Option A', description: 'First option' }, + { label: 'Option B', description: 'Second option' }, + ], + }, + ]; + + const invocation = tool.build({ questions }); + const details = await invocation.shouldConfirmExecute( + new AbortController().signal, + ); + + // Simulate confirmation with empty answers + if (details && 'onConfirm' in details) { + await details.onConfirm(ToolConfirmationOutcome.ProceedOnce, { + answers: {}, + }); + } + + const result = await invocation.execute(new AbortController().signal); + expect(result.returnDisplay).toBe( + 'User submitted without answering questions.', + ); + expect(JSON.parse(result.llmContent as string)).toEqual({ answers: {} }); + }); + + it('should handle cancellation', async () => { + const invocation = tool.build({ + questions: [ + { + question: 'Which sections of the documentation should be updated?', + header: 'Docs', + options: [ + { + label: 'User Guide', + description: 'Update the main user-facing documentation.', + }, + { + label: 'API Reference', + description: 'Update the detailed API documentation.', + }, + ], + multiSelect: true, + }, + ], + }); + + const details = await invocation.shouldConfirmExecute( + new AbortController().signal, + ); + + // Simulate cancellation + if (details && 'onConfirm' in details) { + await details.onConfirm(ToolConfirmationOutcome.Cancel); + } + + const result = await invocation.execute(new AbortController().signal); + expect(result.returnDisplay).toBe('User dismissed dialog'); + expect(result.llmContent).toBe( + 'User dismissed ask_user dialog without answering.', + ); + }); }); }); diff --git a/packages/core/src/tools/ask-user.ts b/packages/core/src/tools/ask-user.ts index 81d62d021c..0e1989967b 100644 --- a/packages/core/src/tools/ask-user.ts +++ b/packages/core/src/tools/ask-user.ts @@ -9,17 +9,12 @@ import { BaseToolInvocation, type ToolResult, Kind, - type ToolCallConfirmationDetails, + type ToolAskUserConfirmationDetails, + type ToolConfirmationPayload, + ToolConfirmationOutcome, } from './tools.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; -import { - MessageBusType, - QuestionType, - type Question, - type AskUserRequest, - type AskUserResponse, -} from '../confirmation-bus/types.js'; -import { randomUUID } from 'node:crypto'; +import { QuestionType, type Question } from '../confirmation-bus/types.js'; import { ASK_USER_TOOL_NAME, ASK_USER_DISPLAY_NAME } from './tool-names.js'; export interface AskUserParams { @@ -165,100 +160,61 @@ export class AskUserInvocation extends BaseToolInvocation< AskUserParams, ToolResult > { + private confirmationOutcome: ToolConfirmationOutcome | null = null; + private userAnswers: { [questionIndex: string]: string } = {}; + override async shouldConfirmExecute( _abortSignal: AbortSignal, - ): Promise { - return false; + ): Promise { + const normalizedQuestions = this.params.questions.map((q) => ({ + ...q, + type: q.type ?? QuestionType.CHOICE, + })); + + return { + type: 'ask_user', + title: 'Ask User', + questions: normalizedQuestions, + onConfirm: async ( + outcome: ToolConfirmationOutcome, + payload?: ToolConfirmationPayload, + ) => { + this.confirmationOutcome = outcome; + if (payload?.answers) { + this.userAnswers = payload.answers; + } + }, + }; } getDescription(): string { return `Asking user: ${this.params.questions.map((q) => q.question).join(', ')}`; } - async execute(signal: AbortSignal): Promise { - const correlationId = randomUUID(); + async execute(_signal: AbortSignal): Promise { + if (this.confirmationOutcome === ToolConfirmationOutcome.Cancel) { + return { + llmContent: 'User dismissed ask_user dialog without answering.', + returnDisplay: 'User dismissed dialog', + }; + } - const request: AskUserRequest = { - type: MessageBusType.ASK_USER_REQUEST, - questions: this.params.questions.map((q) => ({ - ...q, - type: q.type ?? QuestionType.CHOICE, - })), - correlationId, + const answerEntries = Object.entries(this.userAnswers); + const hasAnswers = answerEntries.length > 0; + + const returnDisplay = hasAnswers + ? `**User answered:**\n${answerEntries + .map(([index, answer]) => { + const question = this.params.questions[parseInt(index, 10)]; + const category = question?.header ?? `Q${index}`; + return ` ${category} → ${answer}`; + }) + .join('\n')}` + : 'User submitted without answering questions.'; + + return { + llmContent: JSON.stringify({ answers: this.userAnswers }), + returnDisplay, }; - - return new Promise((resolve, reject) => { - const responseHandler = (response: AskUserResponse): void => { - if (response.correlationId === correlationId) { - cleanup(); - - // Handle user cancellation - if (response.cancelled) { - resolve({ - llmContent: 'User dismissed ask user dialog without answering.', - returnDisplay: 'User dismissed dialog', - }); - return; - } - - // Build formatted key-value display - const answerEntries = Object.entries(response.answers); - const hasAnswers = answerEntries.length > 0; - - const returnDisplay = hasAnswers - ? `**User answered:**\n${answerEntries - .map(([index, answer]) => { - const question = this.params.questions[parseInt(index, 10)]; - const category = question?.header ?? `Q${index}`; - return ` ${category} → ${answer}`; - }) - .join('\n')}` - : 'User submitted without answering questions.'; - - resolve({ - llmContent: JSON.stringify({ answers: response.answers }), - returnDisplay, - }); - } - }; - - const cleanup = () => { - if (responseHandler) { - this.messageBus.unsubscribe( - MessageBusType.ASK_USER_RESPONSE, - responseHandler, - ); - } - signal.removeEventListener('abort', abortHandler); - }; - - const abortHandler = () => { - cleanup(); - resolve({ - llmContent: 'Tool execution cancelled by user.', - returnDisplay: 'Cancelled', - error: { - message: 'Cancelled', - }, - }); - }; - - if (signal.aborted) { - abortHandler(); - return; - } - - signal.addEventListener('abort', abortHandler); - this.messageBus.subscribe( - MessageBusType.ASK_USER_RESPONSE, - responseHandler, - ); - - // Publish request - this.messageBus.publish(request).catch((err) => { - cleanup(); - reject(err); - }); - }); } } diff --git a/packages/core/src/tools/tools.ts b/packages/core/src/tools/tools.ts index 7407ce36da..d95e5246bf 100644 --- a/packages/core/src/tools/tools.ts +++ b/packages/core/src/tools/tools.ts @@ -16,6 +16,7 @@ import { MessageBusType, type ToolConfirmationRequest, type ToolConfirmationResponse, + type Question, } from '../confirmation-bus/types.js'; /** @@ -695,7 +696,9 @@ export interface ToolEditConfirmationDetails { export interface ToolConfirmationPayload { // used to override `modifiedProposedContent` for modifiable tools in the // inline modify flow - newContent: string; + newContent?: string; + // used for askuser tool to return user's answers + answers?: { [questionIndex: string]: string }; } export interface ToolExecuteConfirmationDetails { @@ -725,11 +728,22 @@ export interface ToolInfoConfirmationDetails { urls?: string[]; } +export interface ToolAskUserConfirmationDetails { + type: 'ask_user'; + title: string; + questions: Question[]; + onConfirm: ( + outcome: ToolConfirmationOutcome, + payload?: ToolConfirmationPayload, + ) => Promise; +} + export type ToolCallConfirmationDetails = | ToolEditConfirmationDetails | ToolExecuteConfirmationDetails | ToolMcpConfirmationDetails - | ToolInfoConfirmationDetails; + | ToolInfoConfirmationDetails + | ToolAskUserConfirmationDetails; export enum ToolConfirmationOutcome { ProceedOnce = 'proceed_once',