diff --git a/docs/cli/plan-mode.md b/docs/cli/plan-mode.md index ef41631302..654b54dbc9 100644 --- a/docs/cli/plan-mode.md +++ b/docs/cli/plan-mode.md @@ -119,6 +119,7 @@ These are the only allowed tools: - **Planning (Write):** [`write_file`] and [`replace`] only allowed for `.md` files in the `~/.gemini/tmp///plans/` directory or your [custom plans directory](#custom-plan-directory-and-policies). +- **Memory:** [`save_memory`] - **Skills:** [`activate_skill`] (allows loading specialized instructions and resources in a read-only manner) @@ -277,6 +278,7 @@ performance. You can disable this automatic switching in your settings: [`google_web_search`]: /docs/tools/web-search.md [`replace`]: /docs/tools/file-system.md#6-replace-edit [MCP tools]: /docs/tools/mcp-server.md +[`save_memory`]: /docs/tools/memory.md [`activate_skill`]: /docs/cli/skills.md [subagents]: /docs/core/subagents.md [policy engine]: /docs/reference/policy-engine.md diff --git a/integration-tests/file-system.test.ts b/integration-tests/file-system.test.ts index bdcffedaf8..64481068c2 100644 --- a/integration-tests/file-system.test.ts +++ b/integration-tests/file-system.test.ts @@ -55,8 +55,8 @@ describe('file-system', () => { }); }); - it('should be able to write a file', async () => { - await rig.setup('should be able to write a file', { + it('should be able to write a hello world message to a file', async () => { + await rig.setup('should be able to write a hello world message to a file', { settings: { tools: { core: ['write_file', 'replace', 'read_file'] } }, }); rig.createFile('test.txt', ''); diff --git a/integration-tests/write_file.test.ts b/integration-tests/write_file.test.ts index 8069b1ca87..ece2a11aa4 100644 --- a/integration-tests/write_file.test.ts +++ b/integration-tests/write_file.test.ts @@ -22,8 +22,8 @@ describe('write_file', () => { afterEach(async () => await rig.cleanup()); - it('should be able to write a file', async () => { - await rig.setup('should be able to write a file', { + it('should be able to write a joke to a file', async () => { + await rig.setup('should be able to write a joke to a file', { settings: { tools: { core: ['write_file', 'read_file'] } }, }); const prompt = `show me an example of using the write tool. put a dad joke in dad.txt`; diff --git a/packages/cli/src/test-utils/render.tsx b/packages/cli/src/test-utils/render.tsx index 455a84b8e0..1b64c07d7b 100644 --- a/packages/cli/src/test-utils/render.tsx +++ b/packages/cli/src/test-utils/render.tsx @@ -607,6 +607,7 @@ const mockUIActions: UIActions = { onHintSubmit: vi.fn(), handleRestart: vi.fn(), handleNewAgentsSelect: vi.fn(), + getPreferredEditor: vi.fn(), }; let capturedOverflowState: OverflowState | undefined; diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index 86a4938a66..b89d0b83c0 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -2554,6 +2554,7 @@ Logging in with Google... Restarting Gemini CLI to continue. } setNewAgents(null); }, + getPreferredEditor, }), [ handleThemeSelect, @@ -2605,6 +2606,7 @@ Logging in with Google... Restarting Gemini CLI to continue. newAgents, config, historyManager, + getPreferredEditor, ], ); diff --git a/packages/cli/src/ui/components/AskUserDialog.tsx b/packages/cli/src/ui/components/AskUserDialog.tsx index 1d31b1a1f4..9606513510 100644 --- a/packages/cli/src/ui/components/AskUserDialog.tsx +++ b/packages/cli/src/ui/components/AskUserDialog.tsx @@ -183,6 +183,10 @@ interface AskUserDialogProps { * Height constraint for scrollable content. */ availableHeight?: number; + /** + * Custom keyboard shortcut hints (e.g., ["Ctrl+P to edit"]) + */ + extraParts?: string[]; } interface ReviewViewProps { @@ -190,6 +194,7 @@ interface ReviewViewProps { answers: { [key: string]: string }; onSubmit: () => void; progressHeader?: React.ReactNode; + extraParts?: string[]; } const ReviewView: React.FC = ({ @@ -197,6 +202,7 @@ const ReviewView: React.FC = ({ answers, onSubmit, progressHeader, + extraParts, }) => { const unansweredCount = questions.length - Object.keys(answers).length; const hasUnanswered = unansweredCount > 0; @@ -247,6 +253,7 @@ const ReviewView: React.FC = ({ ); @@ -925,6 +932,7 @@ export const AskUserDialog: React.FC = ({ onActiveTextInputChange, width, availableHeight: availableHeightProp, + extraParts, }) => { const uiState = useContext(UIStateContext); const availableHeight = @@ -1120,6 +1128,7 @@ export const AskUserDialog: React.FC = ({ answers={answers} onSubmit={handleReviewSubmit} progressHeader={progressHeader} + extraParts={extraParts} /> ); @@ -1143,6 +1152,7 @@ export const AskUserDialog: React.FC = ({ ? undefined : '↑/↓ to navigate' } + extraParts={extraParts} /> ); diff --git a/packages/cli/src/ui/components/ExitPlanModeDialog.test.tsx b/packages/cli/src/ui/components/ExitPlanModeDialog.test.tsx index 26b61829a0..c9def1a8c2 100644 --- a/packages/cli/src/ui/components/ExitPlanModeDialog.test.tsx +++ b/packages/cli/src/ui/components/ExitPlanModeDialog.test.tsx @@ -11,6 +11,7 @@ import { waitFor } from '../../test-utils/async.js'; import { ExitPlanModeDialog } from './ExitPlanModeDialog.js'; import { useKeypress } from '../hooks/useKeypress.js'; import { keyMatchers, Command } from '../keyMatchers.js'; +import { openFileInEditor } from '../utils/editorUtils.js'; import { ApprovalMode, validatePlanContent, @@ -19,6 +20,10 @@ import { } from '@google/gemini-cli-core'; import * as fs from 'node:fs'; +vi.mock('../utils/editorUtils.js', () => ({ + openFileInEditor: vi.fn(), +})); + vi.mock('@google/gemini-cli-core', async (importOriginal) => { const actual = await importOriginal(); @@ -144,6 +149,7 @@ Implement a comprehensive authentication system with multiple providers. onApprove={onApprove} onFeedback={onFeedback} onCancel={onCancel} + getPreferredEditor={vi.fn()} width={80} availableHeight={24} />, @@ -153,6 +159,7 @@ Implement a comprehensive authentication system with multiple providers. getTargetDir: () => mockTargetDir, getIdeMode: () => false, isTrustedFolder: () => true, + getPreferredEditor: () => undefined, storage: { getPlansDir: () => mockPlansDir, }, @@ -418,6 +425,7 @@ Implement a comprehensive authentication system with multiple providers. onApprove={onApprove} onFeedback={onFeedback} onCancel={onCancel} + getPreferredEditor={vi.fn()} width={80} availableHeight={24} /> @@ -535,6 +543,40 @@ Implement a comprehensive authentication system with multiple providers. }); expect(onFeedback).not.toHaveBeenCalled(); }); + + it('opens plan in external editor when Ctrl+X is pressed', async () => { + const { stdin, lastFrame } = renderDialog({ useAlternateBuffer }); + + await act(async () => { + vi.runAllTimers(); + }); + + await waitFor(() => { + expect(lastFrame()).toContain('Add user authentication'); + }); + + // Reset the mock to track the second call during refresh + vi.mocked(processSingleFileContent).mockClear(); + + // Press Ctrl+X + await act(async () => { + writeKey(stdin, '\x18'); // Ctrl+X + }); + + await waitFor(() => { + expect(openFileInEditor).toHaveBeenCalledWith( + mockPlanFullPath, + expect.anything(), + expect.anything(), + undefined, + ); + }); + + // Verify that content is refreshed (processSingleFileContent called again) + await waitFor(() => { + expect(processSingleFileContent).toHaveBeenCalled(); + }); + }); }, ); }); diff --git a/packages/cli/src/ui/components/ExitPlanModeDialog.tsx b/packages/cli/src/ui/components/ExitPlanModeDialog.tsx index 8777136d86..6a5da1c299 100644 --- a/packages/cli/src/ui/components/ExitPlanModeDialog.tsx +++ b/packages/cli/src/ui/components/ExitPlanModeDialog.tsx @@ -5,25 +5,32 @@ */ import type React from 'react'; -import { useEffect, useState } from 'react'; -import { Box, Text } from 'ink'; +import { useEffect, useState, useCallback } from 'react'; +import { Box, Text, useStdin } from 'ink'; import { ApprovalMode, validatePlanPath, validatePlanContent, QuestionType, type Config, + type EditorType, processSingleFileContent, + debugLogger, } from '@google/gemini-cli-core'; import { theme } from '../semantic-colors.js'; import { useConfig } from '../contexts/ConfigContext.js'; import { AskUserDialog } from './AskUserDialog.js'; +import { openFileInEditor } from '../utils/editorUtils.js'; +import { useKeypress } from '../hooks/useKeypress.js'; +import { keyMatchers, Command } from '../keyMatchers.js'; +import { formatCommand } from '../utils/keybindingUtils.js'; export interface ExitPlanModeDialogProps { planPath: string; onApprove: (approvalMode: ApprovalMode) => void; onFeedback: (feedback: string) => void; onCancel: () => void; + getPreferredEditor: () => EditorType | undefined; width: number; availableHeight?: number; } @@ -38,6 +45,7 @@ interface PlanContentState { status: PlanStatus; content?: string; error?: string; + refresh: () => void; } enum ApprovalOption { @@ -53,10 +61,15 @@ const StatusMessage: React.FC<{ }> = ({ children }) => {children}; function usePlanContent(planPath: string, config: Config): PlanContentState { - const [state, setState] = useState({ + const [version, setVersion] = useState(0); + const [state, setState] = useState>({ status: PlanStatus.Loading, }); + const refresh = useCallback(() => { + setVersion((v) => v + 1); + }, []); + useEffect(() => { let ignore = false; setState({ status: PlanStatus.Loading }); @@ -120,9 +133,9 @@ function usePlanContent(planPath: string, config: Config): PlanContentState { return () => { ignore = true; }; - }, [planPath, config]); + }, [planPath, config, version]); - return state; + return { ...state, refresh }; } export const ExitPlanModeDialog: React.FC = ({ @@ -130,13 +143,36 @@ export const ExitPlanModeDialog: React.FC = ({ onApprove, onFeedback, onCancel, + getPreferredEditor, width, availableHeight, }) => { const config = useConfig(); + const { stdin, setRawMode } = useStdin(); const planState = usePlanContent(planPath, config); + const { refresh } = planState; const [showLoading, setShowLoading] = useState(false); + const handleOpenEditor = useCallback(async () => { + try { + await openFileInEditor(planPath, stdin, setRawMode, getPreferredEditor()); + refresh(); + } catch (err) { + debugLogger.error('Failed to open plan in editor:', err); + } + }, [planPath, stdin, setRawMode, getPreferredEditor, refresh]); + + useKeypress( + (key) => { + if (keyMatchers[Command.OPEN_EXTERNAL_EDITOR](key)) { + void handleOpenEditor(); + return true; + } + return false; + }, + { isActive: true, priority: true }, + ); + useEffect(() => { if (planState.status !== PlanStatus.Loading) { setShowLoading(false); @@ -183,6 +219,8 @@ export const ExitPlanModeDialog: React.FC = ({ ); } + const editHint = formatCommand(Command.OPEN_EXTERNAL_EDITOR); + return ( = ({ onCancel={onCancel} width={width} availableHeight={availableHeight} + extraParts={[`${editHint} to edit plan`]} /> ); diff --git a/packages/cli/src/ui/components/InputPrompt.test.tsx b/packages/cli/src/ui/components/InputPrompt.test.tsx index bf906d4a80..65a4440d77 100644 --- a/packages/cli/src/ui/components/InputPrompt.test.tsx +++ b/packages/cli/src/ui/components/InputPrompt.test.tsx @@ -411,6 +411,73 @@ describe('InputPrompt', () => { unmount(); }); + it('should submit command in shell mode when Enter pressed with suggestions visible but no arrow navigation', async () => { + props.shellModeActive = true; + props.buffer.setText('ls '); + + mockedUseCommandCompletion.mockReturnValue({ + ...mockCommandCompletion, + showSuggestions: true, + suggestions: [ + { label: 'dir1', value: 'dir1' }, + { label: 'dir2', value: 'dir2' }, + ], + activeSuggestionIndex: 0, + }); + + const { stdin, unmount } = renderWithProviders(, { + uiActions, + }); + + // Press Enter without navigating — should dismiss suggestions and fall + // through to the main submit handler. + await act(async () => { + stdin.write('\r'); + }); + await waitFor(() => { + expect(mockCommandCompletion.resetCompletionState).toHaveBeenCalled(); + expect(props.onSubmit).toHaveBeenCalledWith('ls'); // Assert fall-through (text is trimmed) + }); + expect(mockCommandCompletion.handleAutocomplete).not.toHaveBeenCalled(); + unmount(); + }); + + it('should accept suggestion in shell mode when Enter pressed after arrow navigation', async () => { + props.shellModeActive = true; + props.buffer.setText('ls '); + + mockedUseCommandCompletion.mockReturnValue({ + ...mockCommandCompletion, + showSuggestions: true, + suggestions: [ + { label: 'dir1', value: 'dir1' }, + { label: 'dir2', value: 'dir2' }, + ], + activeSuggestionIndex: 1, + }); + + const { stdin, unmount } = renderWithProviders(, { + uiActions, + }); + + // Press ArrowDown to navigate, then Enter to accept + await act(async () => { + stdin.write('\u001B[B'); // ArrowDown — sets hasUserNavigatedSuggestions + }); + await waitFor(() => + expect(mockCommandCompletion.navigateDown).toHaveBeenCalled(), + ); + + await act(async () => { + stdin.write('\r'); // Enter — should accept navigated suggestion + }); + await waitFor(() => { + expect(mockCommandCompletion.handleAutocomplete).toHaveBeenCalledWith(1); + }); + expect(props.onSubmit).not.toHaveBeenCalled(); + unmount(); + }); + it('should NOT call shell history methods when not in shell mode', async () => { props.buffer.setText('some text'); const { stdin, unmount } = renderWithProviders(, { diff --git a/packages/cli/src/ui/components/InputPrompt.tsx b/packages/cli/src/ui/components/InputPrompt.tsx index ad84dd27f6..ac17284189 100644 --- a/packages/cli/src/ui/components/InputPrompt.tsx +++ b/packages/cli/src/ui/components/InputPrompt.tsx @@ -254,6 +254,7 @@ export const InputPrompt: React.FC = ({ >(null); const pasteTimeoutRef = useRef(null); const innerBoxRef = useRef(null); + const hasUserNavigatedSuggestions = useRef(false); const [reverseSearchActive, setReverseSearchActive] = useState(false); const [commandSearchActive, setCommandSearchActive] = useState(false); @@ -610,6 +611,7 @@ export const InputPrompt: React.FC = ({ setSuppressCompletion( isHistoryNav || isCursorMovement || keyMatchers[Command.ESCAPE](key), ); + hasUserNavigatedSuggestions.current = false; } // TODO(jacobr): this special case is likely not needed anymore. @@ -643,7 +645,13 @@ export const InputPrompt: React.FC = ({ Boolean(completion.promptCompletion.text) || reverseSearchActive || commandSearchActive; - if (isPlainTab) { + + if (isPlainTab && shellModeActive) { + resetPlainTabPress(); + if (!completion.showSuggestions) { + setSuppressCompletion(false); + } + } else if (isPlainTab) { if (!hasTabCompletionInteraction) { if (registerPlainTabPress() === 2) { toggleCleanUiDetailsVisible(); @@ -903,11 +911,13 @@ export const InputPrompt: React.FC = ({ if (completion.suggestions.length > 1) { if (keyMatchers[Command.COMPLETION_UP](key)) { completion.navigateUp(); + hasUserNavigatedSuggestions.current = true; setExpandedSuggestionIndex(-1); // Reset expansion when navigating return true; } if (keyMatchers[Command.COMPLETION_DOWN](key)) { completion.navigateDown(); + hasUserNavigatedSuggestions.current = true; setExpandedSuggestionIndex(-1); // Reset expansion when navigating return true; } @@ -925,6 +935,24 @@ export const InputPrompt: React.FC = ({ const isEnterKey = key.name === 'return' && !key.ctrl; + if (isEnterKey && shellModeActive) { + if (hasUserNavigatedSuggestions.current) { + completion.handleAutocomplete( + completion.activeSuggestionIndex, + ); + setExpandedSuggestionIndex(-1); + hasUserNavigatedSuggestions.current = false; + return true; + } + completion.resetCompletionState(); + setExpandedSuggestionIndex(-1); + hasUserNavigatedSuggestions.current = false; + if (buffer.text.trim()) { + handleSubmit(buffer.text); + } + return true; + } + if (isEnterKey && buffer.text.startsWith('/')) { const { isArgumentCompletion, leafCommand } = completion.slashCompletionRange; @@ -1381,7 +1409,8 @@ export const InputPrompt: React.FC = ({ scrollOffset={activeCompletion.visibleStartIndex} userInput={buffer.text} mode={ - completion.completionMode === CompletionMode.AT + completion.completionMode === CompletionMode.AT || + completion.completionMode === CompletionMode.SHELL ? 'reverse' : buffer.text.startsWith('/') && !reverseSearchActive && diff --git a/packages/cli/src/ui/components/ToolConfirmationQueue.tsx b/packages/cli/src/ui/components/ToolConfirmationQueue.tsx index c89c98f8d4..3fb1cc8c6f 100644 --- a/packages/cli/src/ui/components/ToolConfirmationQueue.tsx +++ b/packages/cli/src/ui/components/ToolConfirmationQueue.tsx @@ -17,6 +17,7 @@ import { ShowMoreLines } from './ShowMoreLines.js'; import { StickyHeader } from './StickyHeader.js'; import { useAlternateBuffer } from '../hooks/useAlternateBuffer.js'; import type { SerializableConfirmationDetails } from '@google/gemini-cli-core'; +import { useUIActions } from '../contexts/UIActionsContext.js'; function getConfirmationHeader( details: SerializableConfirmationDetails | undefined, @@ -41,6 +42,7 @@ export const ToolConfirmationQueue: React.FC = ({ confirmingTool, }) => { const config = useConfig(); + const { getPreferredEditor } = useUIActions(); const isAlternateBuffer = useAlternateBuffer(); const { mainAreaWidth, @@ -134,6 +136,7 @@ export const ToolConfirmationQueue: React.FC = ({ callId={tool.callId} confirmationDetails={tool.confirmationDetails} config={config} + getPreferredEditor={getPreferredEditor} terminalWidth={mainAreaWidth - 4} // Adjust for parent border/padding availableTerminalHeight={availableContentHeight} isFocused={true} diff --git a/packages/cli/src/ui/components/__snapshots__/ExitPlanModeDialog.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/ExitPlanModeDialog.test.tsx.snap index 587ded8f29..0cd4553c77 100644 --- a/packages/cli/src/ui/components/__snapshots__/ExitPlanModeDialog.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/ExitPlanModeDialog.test.tsx.snap @@ -23,7 +23,7 @@ Files to Modify Approves plan but requires confirmation for each tool 3. Type your feedback... -Enter to select · ↑/↓ to navigate · Esc to cancel +Enter to select · ↑/↓ to navigate · Ctrl+X to edit plan · Esc to cancel " `; @@ -50,7 +50,7 @@ Files to Modify Approves plan but requires confirmation for each tool 3. Type your feedback... -Enter to select · ↑/↓ to navigate · Esc to cancel +Enter to select · ↑/↓ to navigate · Ctrl+X to edit plan · Esc to cancel " `; @@ -82,7 +82,7 @@ Implementation Steps Approves plan but requires confirmation for each tool 3. Type your feedback... -Enter to select · ↑/↓ to navigate · Esc to cancel +Enter to select · ↑/↓ to navigate · Ctrl+X to edit plan · Esc to cancel " `; @@ -109,7 +109,7 @@ Files to Modify Approves plan but requires confirmation for each tool 3. Type your feedback... -Enter to select · ↑/↓ to navigate · Esc to cancel +Enter to select · ↑/↓ to navigate · Ctrl+X to edit plan · Esc to cancel " `; @@ -136,7 +136,7 @@ Files to Modify Approves plan but requires confirmation for each tool 3. Type your feedback... -Enter to select · ↑/↓ to navigate · Esc to cancel +Enter to select · ↑/↓ to navigate · Ctrl+X to edit plan · Esc to cancel " `; @@ -163,7 +163,7 @@ Files to Modify Approves plan but requires confirmation for each tool 3. Type your feedback... -Enter to select · ↑/↓ to navigate · Esc to cancel +Enter to select · ↑/↓ to navigate · Ctrl+X to edit plan · Esc to cancel " `; @@ -216,7 +216,7 @@ Testing Strategy Approves plan but requires confirmation for each tool 3. Type your feedback... -Enter to select · ↑/↓ to navigate · Esc to cancel +Enter to select · ↑/↓ to navigate · Ctrl+X to edit plan · Esc to cancel " `; @@ -243,6 +243,6 @@ Files to Modify Approves plan but requires confirmation for each tool 3. Type your feedback... -Enter to select · ↑/↓ to navigate · Esc to cancel +Enter to select · ↑/↓ to navigate · Ctrl+X to edit plan · 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 b5e013ef48..ad7e046465 100644 --- a/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue.test.tsx.snap @@ -85,7 +85,7 @@ exports[`ToolConfirmationQueue > renders ExitPlanMode tool confirmation with Suc │ Approves plan but requires confirmation for each tool │ │ 3. Type your feedback... │ │ │ -│ Enter to select · ↑/↓ to navigate · Esc to cancel │ +│ Enter to select · ↑/↓ to navigate · Ctrl+X to edit plan · Esc to cancel │ ╰──────────────────────────────────────────────────────────────────────────────╯ " `; diff --git a/packages/cli/src/ui/components/messages/RedirectionConfirmation.test.tsx b/packages/cli/src/ui/components/messages/RedirectionConfirmation.test.tsx index 1c95a526f5..15763bdae7 100644 --- a/packages/cli/src/ui/components/messages/RedirectionConfirmation.test.tsx +++ b/packages/cli/src/ui/components/messages/RedirectionConfirmation.test.tsx @@ -37,6 +37,7 @@ describe('ToolConfirmationMessage Redirection', () => { callId="test-call-id" confirmationDetails={confirmationDetails} config={mockConfig} + getPreferredEditor={vi.fn()} availableTerminalHeight={30} terminalWidth={100} />, diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx index ec1fd3d4db..b3b34ae0a8 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx @@ -52,6 +52,7 @@ describe('ToolConfirmationMessage', () => { callId="test-call-id" confirmationDetails={confirmationDetails} config={mockConfig} + getPreferredEditor={vi.fn()} availableTerminalHeight={30} terminalWidth={80} />, @@ -78,6 +79,7 @@ describe('ToolConfirmationMessage', () => { callId="test-call-id" confirmationDetails={confirmationDetails} config={mockConfig} + getPreferredEditor={vi.fn()} availableTerminalHeight={30} terminalWidth={80} />, @@ -101,6 +103,7 @@ describe('ToolConfirmationMessage', () => { callId="test-call-id" confirmationDetails={confirmationDetails} config={mockConfig} + getPreferredEditor={vi.fn()} availableTerminalHeight={30} terminalWidth={80} />, @@ -131,6 +134,7 @@ describe('ToolConfirmationMessage', () => { callId="test-call-id" confirmationDetails={confirmationDetails} config={mockConfig} + getPreferredEditor={vi.fn()} availableTerminalHeight={30} terminalWidth={80} />, @@ -161,6 +165,7 @@ describe('ToolConfirmationMessage', () => { callId="test-call-id" confirmationDetails={confirmationDetails} config={mockConfig} + getPreferredEditor={vi.fn()} availableTerminalHeight={30} terminalWidth={80} />, @@ -190,6 +195,7 @@ describe('ToolConfirmationMessage', () => { callId="test-call-id" confirmationDetails={confirmationDetails} config={mockConfig} + getPreferredEditor={vi.fn()} availableTerminalHeight={30} terminalWidth={80} />, @@ -219,6 +225,7 @@ describe('ToolConfirmationMessage', () => { callId="test-call-id" confirmationDetails={confirmationDetails} config={mockConfig} + getPreferredEditor={vi.fn()} availableTerminalHeight={30} terminalWidth={80} />, @@ -300,6 +307,7 @@ describe('ToolConfirmationMessage', () => { callId="test-call-id" confirmationDetails={details} config={mockConfig} + getPreferredEditor={vi.fn()} availableTerminalHeight={30} terminalWidth={80} />, @@ -321,6 +329,7 @@ describe('ToolConfirmationMessage', () => { callId="test-call-id" confirmationDetails={details} config={mockConfig} + getPreferredEditor={vi.fn()} availableTerminalHeight={30} terminalWidth={80} />, @@ -355,6 +364,7 @@ describe('ToolConfirmationMessage', () => { callId="test-call-id" confirmationDetails={editConfirmationDetails} config={mockConfig} + getPreferredEditor={vi.fn()} availableTerminalHeight={30} terminalWidth={80} />, @@ -381,6 +391,7 @@ describe('ToolConfirmationMessage', () => { callId="test-call-id" confirmationDetails={editConfirmationDetails} config={mockConfig} + getPreferredEditor={vi.fn()} availableTerminalHeight={30} terminalWidth={80} />, @@ -425,6 +436,7 @@ describe('ToolConfirmationMessage', () => { callId="test-call-id" confirmationDetails={editConfirmationDetails} config={mockConfig} + getPreferredEditor={vi.fn()} availableTerminalHeight={30} terminalWidth={80} />, @@ -452,6 +464,7 @@ describe('ToolConfirmationMessage', () => { callId="test-call-id" confirmationDetails={editConfirmationDetails} config={mockConfig} + getPreferredEditor={vi.fn()} availableTerminalHeight={30} terminalWidth={80} />, @@ -479,6 +492,7 @@ describe('ToolConfirmationMessage', () => { callId="test-call-id" confirmationDetails={editConfirmationDetails} config={mockConfig} + getPreferredEditor={vi.fn()} availableTerminalHeight={30} terminalWidth={80} />, @@ -505,6 +519,7 @@ describe('ToolConfirmationMessage', () => { callId="test-call-id" confirmationDetails={confirmationDetails} config={mockConfig} + getPreferredEditor={vi.fn()} availableTerminalHeight={30} terminalWidth={80} />, @@ -550,6 +565,7 @@ describe('ToolConfirmationMessage', () => { callId="test-call-id" confirmationDetails={confirmationDetails} config={mockConfig} + getPreferredEditor={vi.fn()} availableTerminalHeight={30} terminalWidth={80} />, @@ -581,6 +597,7 @@ describe('ToolConfirmationMessage', () => { callId="test-call-id" confirmationDetails={confirmationDetails} config={mockConfig} + getPreferredEditor={vi.fn()} availableTerminalHeight={30} terminalWidth={80} />, diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx index 9a49e2aa5a..022a68e953 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx @@ -14,6 +14,7 @@ import { type Config, type ToolConfirmationPayload, ToolConfirmationOutcome, + type EditorType, hasRedirection, debugLogger, } from '@google/gemini-cli-core'; @@ -49,6 +50,7 @@ export interface ToolConfirmationMessageProps { callId: string; confirmationDetails: SerializableConfirmationDetails; config: Config; + getPreferredEditor: () => EditorType | undefined; isFocused?: boolean; availableTerminalHeight?: number; terminalWidth: number; @@ -60,6 +62,7 @@ export const ToolConfirmationMessage: React.FC< callId, confirmationDetails, config, + getPreferredEditor, isFocused = true, availableTerminalHeight, terminalWidth, @@ -424,6 +427,7 @@ export const ToolConfirmationMessage: React.FC< bodyContent = ( { handleConfirm(ToolConfirmationOutcome.ProceedOnce, { approved: true, @@ -629,6 +633,7 @@ export const ToolConfirmationMessage: React.FC< hasMcpToolDetails, mcpToolDetailsText, expandDetailsHintKey, + getPreferredEditor, ]); const bodyOverflowDirection: 'top' | 'bottom' = diff --git a/packages/cli/src/ui/components/shared/DialogFooter.tsx b/packages/cli/src/ui/components/shared/DialogFooter.tsx index af75074645..7411a91611 100644 --- a/packages/cli/src/ui/components/shared/DialogFooter.tsx +++ b/packages/cli/src/ui/components/shared/DialogFooter.tsx @@ -15,6 +15,8 @@ export interface DialogFooterProps { navigationActions?: string; /** Exit shortcut (defaults to "Esc to cancel") */ cancelAction?: string; + /** Custom keyboard shortcut hints (e.g., ["Ctrl+P to edit"]) */ + extraParts?: string[]; } /** @@ -25,11 +27,13 @@ export const DialogFooter: React.FC = ({ primaryAction, navigationActions, cancelAction = 'Esc to cancel', + extraParts = [], }) => { const parts = [primaryAction]; if (navigationActions) { parts.push(navigationActions); } + parts.push(...extraParts); parts.push(cancelAction); return ( diff --git a/packages/cli/src/ui/components/shared/text-buffer.ts b/packages/cli/src/ui/components/shared/text-buffer.ts index e641633e97..71ee40b642 100644 --- a/packages/cli/src/ui/components/shared/text-buffer.ts +++ b/packages/cli/src/ui/components/shared/text-buffer.ts @@ -4,7 +4,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { spawnSync } from 'node:child_process'; import fs from 'node:fs'; import os from 'node:os'; import pathMod from 'node:path'; @@ -13,12 +12,9 @@ import { useState, useCallback, useEffect, useMemo, useReducer } from 'react'; import { LRUCache } from 'mnemonist'; import { coreEvents, - CoreEvent, debugLogger, unescapePath, type EditorType, - getEditorCommand, - isGuiEditor, } from '@google/gemini-cli-core'; import { toCodePoints, @@ -33,6 +29,7 @@ import { keyMatchers, Command } from '../../keyMatchers.js'; import type { VimAction } from './vim-buffer-actions.js'; import { handleVimAction } from './vim-buffer-actions.js'; import { LRU_BUFFER_PERF_CACHE_LIMIT } from '../../constants.js'; +import { openFileInEditor } from '../../utils/editorUtils.js'; export const LARGE_PASTE_LINE_THRESHOLD = 5; export const LARGE_PASTE_CHAR_THRESHOLD = 500; @@ -3095,36 +3092,15 @@ export function useTextBuffer({ ); fs.writeFileSync(filePath, expandedText, 'utf8'); - let command: string | undefined = undefined; - const args = [filePath]; - - const preferredEditorType = getPreferredEditor?.(); - if (!command && preferredEditorType) { - command = getEditorCommand(preferredEditorType); - if (isGuiEditor(preferredEditorType)) { - args.unshift('--wait'); - } - } - - if (!command) { - command = - process.env['VISUAL'] ?? - process.env['EDITOR'] ?? - (process.platform === 'win32' ? 'notepad' : 'vi'); - } - dispatch({ type: 'create_undo_snapshot' }); - const wasRaw = stdin?.isRaw ?? false; try { - setRawMode?.(false); - const { status, error } = spawnSync(command, args, { - stdio: 'inherit', - shell: process.platform === 'win32', - }); - if (error) throw error; - if (typeof status === 'number' && status !== 0) - throw new Error(`External editor exited with status ${status}`); + await openFileInEditor( + filePath, + stdin, + setRawMode, + getPreferredEditor?.(), + ); let newText = fs.readFileSync(filePath, 'utf8'); newText = newText.replace(/\r\n?/g, '\n'); @@ -3147,8 +3123,6 @@ export function useTextBuffer({ err, ); } finally { - coreEvents.emit(CoreEvent.ExternalEditorClosed); - if (wasRaw) setRawMode?.(true); try { fs.unlinkSync(filePath); } catch { diff --git a/packages/cli/src/ui/contexts/UIActionsContext.tsx b/packages/cli/src/ui/contexts/UIActionsContext.tsx index 03780c5068..23c5e995db 100644 --- a/packages/cli/src/ui/contexts/UIActionsContext.tsx +++ b/packages/cli/src/ui/contexts/UIActionsContext.tsx @@ -87,6 +87,7 @@ export interface UIActions { onHintSubmit: (hint: string) => void; handleRestart: () => void; handleNewAgentsSelect: (choice: NewAgentsChoice) => Promise; + getPreferredEditor: () => EditorType | undefined; } export const UIActionsContext = createContext(null); diff --git a/packages/cli/src/ui/hooks/shell-completions/gitProvider.test.ts b/packages/cli/src/ui/hooks/shell-completions/gitProvider.test.ts new file mode 100644 index 0000000000..f0d7cb4573 --- /dev/null +++ b/packages/cli/src/ui/hooks/shell-completions/gitProvider.test.ts @@ -0,0 +1,131 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { gitProvider } from './gitProvider.js'; +import * as childProcess from 'node:child_process'; + +vi.mock('node:child_process', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + execFile: vi.fn(), + }; +}); + +describe('gitProvider', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('suggests git subcommands for cursorIndex 1', async () => { + const result = await gitProvider.getCompletions(['git', 'ch'], 1, '/tmp'); + + expect(result.exclusive).toBe(true); + expect(result.suggestions).toEqual( + expect.arrayContaining([expect.objectContaining({ value: 'checkout' })]), + ); + expect( + result.suggestions.find((s) => s.value === 'commit'), + ).toBeUndefined(); + }); + + it('suggests branch names for checkout at cursorIndex 2', async () => { + vi.mocked(childProcess.execFile).mockImplementation( + (_cmd, _args, _opts, cb: unknown) => { + const callback = (typeof _opts === 'function' ? _opts : cb) as ( + error: Error | null, + result: { stdout: string }, + ) => void; + callback(null, { + stdout: 'main\nfeature-branch\nfix/bug\nbranch(with)special\n', + }); + return {} as ReturnType; + }, + ); + + const result = await gitProvider.getCompletions( + ['git', 'checkout', 'feat'], + 2, + '/tmp', + ); + + expect(result.exclusive).toBe(true); + expect(result.suggestions).toHaveLength(1); + expect(result.suggestions[0].label).toBe('feature-branch'); + expect(result.suggestions[0].value).toBe('feature-branch'); + expect(childProcess.execFile).toHaveBeenCalledWith( + 'git', + ['branch', '--format=%(refname:short)'], + expect.any(Object), + expect.any(Function), + ); + }); + + it('escapes branch names with shell metacharacters', async () => { + vi.mocked(childProcess.execFile).mockImplementation( + (_cmd, _args, _opts, cb: unknown) => { + const callback = (typeof _opts === 'function' ? _opts : cb) as ( + error: Error | null, + result: { stdout: string }, + ) => void; + callback(null, { stdout: 'main\nbranch(with)special\n' }); + return {} as ReturnType; + }, + ); + + const result = await gitProvider.getCompletions( + ['git', 'checkout', 'branch('], + 2, + '/tmp', + ); + + expect(result.exclusive).toBe(true); + expect(result.suggestions).toHaveLength(1); + expect(result.suggestions[0].label).toBe('branch(with)special'); + + // On Windows, space escape is not done. But since UNIX_SHELL_SPECIAL_CHARS is mostly tested, + // we can use a matcher that checks if escaping was applied (it differs per platform but that's handled by escapeShellPath). + // Let's match the value against either unescaped (win) or escaped (unix). + const isWin = process.platform === 'win32'; + expect(result.suggestions[0].value).toBe( + isWin ? 'branch(with)special' : 'branch\\(with\\)special', + ); + }); + + it('returns empty results if git branch fails', async () => { + vi.mocked(childProcess.execFile).mockImplementation( + (_cmd, _args, _opts, cb: unknown) => { + const callback = (typeof _opts === 'function' ? _opts : cb) as ( + error: Error, + stdout?: string, + ) => void; + callback(new Error('Not a git repository')); + return {} as ReturnType; + }, + ); + + const result = await gitProvider.getCompletions( + ['git', 'checkout', ''], + 2, + '/tmp', + ); + + expect(result.exclusive).toBe(true); + expect(result.suggestions).toHaveLength(0); + }); + + it('returns non-exclusive for unrecognized position', async () => { + const result = await gitProvider.getCompletions( + ['git', 'commit', '-m', 'some message'], + 3, + '/tmp', + ); + + expect(result.exclusive).toBe(false); + expect(result.suggestions).toHaveLength(0); + }); +}); diff --git a/packages/cli/src/ui/hooks/shell-completions/gitProvider.ts b/packages/cli/src/ui/hooks/shell-completions/gitProvider.ts new file mode 100644 index 0000000000..7115718487 --- /dev/null +++ b/packages/cli/src/ui/hooks/shell-completions/gitProvider.ts @@ -0,0 +1,93 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { execFile } from 'node:child_process'; +import { promisify } from 'node:util'; +import type { ShellCompletionProvider, CompletionResult } from './types.js'; +import { escapeShellPath } from '../useShellCompletion.js'; + +const execFileAsync = promisify(execFile); + +const GIT_SUBCOMMANDS = [ + 'add', + 'branch', + 'checkout', + 'commit', + 'diff', + 'merge', + 'pull', + 'push', + 'rebase', + 'status', + 'switch', +]; + +export const gitProvider: ShellCompletionProvider = { + command: 'git', + async getCompletions( + tokens: string[], + cursorIndex: number, + cwd: string, + signal?: AbortSignal, + ): Promise { + // We are completing the first argument (subcommand) + if (cursorIndex === 1) { + const partial = tokens[1] || ''; + return { + suggestions: GIT_SUBCOMMANDS.filter((cmd) => + cmd.startsWith(partial), + ).map((cmd) => ({ + label: cmd, + value: cmd, + description: 'git command', + })), + exclusive: true, + }; + } + + // We are completing the second argument (e.g. branch name) + if (cursorIndex === 2) { + const subcommand = tokens[1]; + if ( + subcommand === 'checkout' || + subcommand === 'switch' || + subcommand === 'merge' || + subcommand === 'branch' + ) { + const partial = tokens[2] || ''; + try { + const { stdout } = await execFileAsync( + 'git', + ['branch', '--format=%(refname:short)'], + { cwd, signal }, + ); + + const branches = stdout + .split('\n') + .map((b) => b.trim()) + .filter(Boolean); + + return { + suggestions: branches + .filter((b) => b.startsWith(partial)) + .map((b) => ({ + label: b, + value: escapeShellPath(b), + description: 'branch', + })), + exclusive: true, + }; + } catch { + // If git fails (e.g. not a git repo), return nothing + return { suggestions: [], exclusive: true }; + } + } + } + + // Unhandled git argument, fallback to default file completions + return { suggestions: [], exclusive: false }; + }, +}; diff --git a/packages/cli/src/ui/hooks/shell-completions/index.ts b/packages/cli/src/ui/hooks/shell-completions/index.ts new file mode 100644 index 0000000000..07b38abda6 --- /dev/null +++ b/packages/cli/src/ui/hooks/shell-completions/index.ts @@ -0,0 +1,25 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { ShellCompletionProvider, CompletionResult } from './types.js'; +import { gitProvider } from './gitProvider.js'; +import { npmProvider } from './npmProvider.js'; + +const providers: ShellCompletionProvider[] = [gitProvider, npmProvider]; + +export async function getArgumentCompletions( + commandToken: string, + tokens: string[], + cursorIndex: number, + cwd: string, + signal?: AbortSignal, +): Promise { + const provider = providers.find((p) => p.command === commandToken); + if (!provider) { + return null; + } + return provider.getCompletions(tokens, cursorIndex, cwd, signal); +} diff --git a/packages/cli/src/ui/hooks/shell-completions/npmProvider.test.ts b/packages/cli/src/ui/hooks/shell-completions/npmProvider.test.ts new file mode 100644 index 0000000000..95e61b7015 --- /dev/null +++ b/packages/cli/src/ui/hooks/shell-completions/npmProvider.test.ts @@ -0,0 +1,106 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { npmProvider } from './npmProvider.js'; +import * as fs from 'node:fs/promises'; + +vi.mock('node:fs/promises', () => ({ + readFile: vi.fn(), +})); + +describe('npmProvider', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('suggests npm subcommands for cursorIndex 1', async () => { + const result = await npmProvider.getCompletions(['npm', 'ru'], 1, '/tmp'); + + expect(result.exclusive).toBe(true); + expect(result.suggestions).toEqual([ + expect.objectContaining({ value: 'run' }), + ]); + }); + + it('suggests package.json scripts for npm run at cursorIndex 2', async () => { + const mockPackageJson = { + scripts: { + start: 'node index.js', + build: 'tsc', + 'build:dev': 'tsc --watch', + }, + }; + vi.mocked(fs.readFile).mockResolvedValue(JSON.stringify(mockPackageJson)); + + const result = await npmProvider.getCompletions( + ['npm', 'run', 'bu'], + 2, + '/tmp', + ); + + expect(result.exclusive).toBe(true); + expect(result.suggestions).toHaveLength(2); + expect(result.suggestions[0].label).toBe('build'); + expect(result.suggestions[0].value).toBe('build'); + expect(result.suggestions[1].label).toBe('build:dev'); + expect(result.suggestions[1].value).toBe('build:dev'); + expect(fs.readFile).toHaveBeenCalledWith( + expect.stringContaining('package.json'), + 'utf8', + ); + }); + + it('escapes script names with shell metacharacters', async () => { + const mockPackageJson = { + scripts: { + 'build(prod)': 'tsc', + 'test:watch': 'vitest', + }, + }; + vi.mocked(fs.readFile).mockResolvedValue(JSON.stringify(mockPackageJson)); + + const result = await npmProvider.getCompletions( + ['npm', 'run', 'bu'], + 2, + '/tmp', + ); + + expect(result.exclusive).toBe(true); + expect(result.suggestions).toHaveLength(1); + expect(result.suggestions[0].label).toBe('build(prod)'); + + // Windows does not escape spaces/parens in cmds by default in our function, but Unix does. + const isWin = process.platform === 'win32'; + expect(result.suggestions[0].value).toBe( + isWin ? 'build(prod)' : 'build\\(prod\\)', + ); + }); + + it('handles missing package.json gracefully', async () => { + vi.mocked(fs.readFile).mockRejectedValue(new Error('ENOENT')); + + const result = await npmProvider.getCompletions( + ['npm', 'run', ''], + 2, + '/tmp', + ); + + expect(result.exclusive).toBe(true); + expect(result.suggestions).toHaveLength(0); + }); + + it('returns non-exclusive for unrecognized position', async () => { + const result = await npmProvider.getCompletions( + ['npm', 'install', 'react'], + 2, + '/tmp', + ); + + expect(result.exclusive).toBe(false); + expect(result.suggestions).toHaveLength(0); + }); +}); diff --git a/packages/cli/src/ui/hooks/shell-completions/npmProvider.ts b/packages/cli/src/ui/hooks/shell-completions/npmProvider.ts new file mode 100644 index 0000000000..32b88ca5ca --- /dev/null +++ b/packages/cli/src/ui/hooks/shell-completions/npmProvider.ts @@ -0,0 +1,81 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as fs from 'node:fs/promises'; +import * as path from 'node:path'; +import type { ShellCompletionProvider, CompletionResult } from './types.js'; +import { escapeShellPath } from '../useShellCompletion.js'; + +const NPM_SUBCOMMANDS = [ + 'build', + 'ci', + 'dev', + 'install', + 'publish', + 'run', + 'start', + 'test', +]; + +export const npmProvider: ShellCompletionProvider = { + command: 'npm', + async getCompletions( + tokens: string[], + cursorIndex: number, + cwd: string, + signal?: AbortSignal, + ): Promise { + if (cursorIndex === 1) { + const partial = tokens[1] || ''; + return { + suggestions: NPM_SUBCOMMANDS.filter((cmd) => + cmd.startsWith(partial), + ).map((cmd) => ({ + label: cmd, + value: cmd, + description: 'npm command', + })), + exclusive: true, + }; + } + + if (cursorIndex === 2 && tokens[1] === 'run') { + const partial = tokens[2] || ''; + try { + if (signal?.aborted) return { suggestions: [], exclusive: true }; + + const pkgJsonPath = path.join(cwd, 'package.json'); + const content = await fs.readFile(pkgJsonPath, 'utf8'); + const pkg = JSON.parse(content) as unknown; + + const scripts = + pkg && + typeof pkg === 'object' && + 'scripts' in pkg && + pkg.scripts && + typeof pkg.scripts === 'object' + ? Object.keys(pkg.scripts) + : []; + + return { + suggestions: scripts + .filter((s) => s.startsWith(partial)) + .map((s) => ({ + label: s, + value: escapeShellPath(s), + description: 'npm script', + })), + exclusive: true, + }; + } catch { + // No package.json or invalid JSON + return { suggestions: [], exclusive: true }; + } + } + + return { suggestions: [], exclusive: false }; + }, +}; diff --git a/packages/cli/src/ui/hooks/shell-completions/types.ts b/packages/cli/src/ui/hooks/shell-completions/types.ts new file mode 100644 index 0000000000..df3900cf8f --- /dev/null +++ b/packages/cli/src/ui/hooks/shell-completions/types.ts @@ -0,0 +1,24 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { Suggestion } from '../../components/SuggestionsDisplay.js'; + +export interface CompletionResult { + suggestions: Suggestion[]; + // If true, this prevents the shell from appending generic file/path completions + // to this list. Use this when the tool expects ONLY specific values (e.g. branches). + exclusive?: boolean; +} + +export interface ShellCompletionProvider { + command: string; // The command trigger, e.g., 'git' or 'npm' + getCompletions( + tokens: string[], // List of arguments parsed from the input + cursorIndex: number, // Which token index the cursor is currently on + cwd: string, + signal?: AbortSignal, + ): Promise; +} diff --git a/packages/cli/src/ui/hooks/useCommandCompletion.test.tsx b/packages/cli/src/ui/hooks/useCommandCompletion.test.tsx index 2b0bad2743..c7bb2afb50 100644 --- a/packages/cli/src/ui/hooks/useCommandCompletion.test.tsx +++ b/packages/cli/src/ui/hooks/useCommandCompletion.test.tsx @@ -40,6 +40,16 @@ vi.mock('./useSlashCompletion', () => ({ })), })); +vi.mock('./useShellCompletion', async () => { + const actual = await vi.importActual< + typeof import('./useShellCompletion.js') + >('./useShellCompletion'); + return { + ...actual, + useShellCompletion: vi.fn(), + }; +}); + // Helper to set up mocks in a consistent way for both child hooks const setupMocks = ({ atSuggestions = [], @@ -94,6 +104,7 @@ const setupMocks = ({ describe('useCommandCompletion', () => { const mockCommandContext = {} as CommandContext; const mockConfig = { + getEnablePromptCompletion: () => false, getGeminiClient: vi.fn(), } as unknown as Config; const testRootDir = '/'; @@ -498,6 +509,7 @@ describe('useCommandCompletion', () => { describe('prompt completion filtering', () => { it('should not trigger prompt completion for line comments', async () => { const mockConfig = { + getEnablePromptCompletion: () => true, getGeminiClient: vi.fn(), } as unknown as Config; @@ -530,6 +542,7 @@ describe('useCommandCompletion', () => { it('should not trigger prompt completion for block comments', async () => { const mockConfig = { + getEnablePromptCompletion: () => true, getGeminiClient: vi.fn(), } as unknown as Config; @@ -564,6 +577,7 @@ describe('useCommandCompletion', () => { it('should trigger prompt completion for regular text when enabled', async () => { const mockConfig = { + getEnablePromptCompletion: () => true, getGeminiClient: vi.fn(), } as unknown as Config; diff --git a/packages/cli/src/ui/hooks/useCommandCompletion.tsx b/packages/cli/src/ui/hooks/useCommandCompletion.tsx index b9fcb95626..097a1e63b3 100644 --- a/packages/cli/src/ui/hooks/useCommandCompletion.tsx +++ b/packages/cli/src/ui/hooks/useCommandCompletion.tsx @@ -13,6 +13,7 @@ import { isSlashCommand } from '../utils/commandUtils.js'; import { toCodePoints } from '../utils/textUtils.js'; import { useAtCompletion } from './useAtCompletion.js'; import { useSlashCompletion } from './useSlashCompletion.js'; +import { useShellCompletion, getTokenAtCursor } from './useShellCompletion.js'; import type { PromptCompletion } from './usePromptCompletion.js'; import { usePromptCompletion, @@ -26,6 +27,7 @@ export enum CompletionMode { AT = 'AT', SLASH = 'SLASH', PROMPT = 'PROMPT', + SHELL = 'SHELL', } export interface UseCommandCompletionReturn { @@ -99,85 +101,135 @@ export function useCommandCompletion({ const cursorRow = buffer.cursor[0]; const cursorCol = buffer.cursor[1]; - const { completionMode, query, completionStart, completionEnd } = - useMemo(() => { - const currentLine = buffer.lines[cursorRow] || ''; - const codePoints = toCodePoints(currentLine); + const { + completionMode, + query, + completionStart, + completionEnd, + shellTokenIsCommand, + shellTokens, + shellCursorIndex, + shellCommandToken, + } = useMemo(() => { + const currentLine = buffer.lines[cursorRow] || ''; + const codePoints = toCodePoints(currentLine); - // FIRST: Check for @ completion (scan backwards from cursor) - // This must happen before slash command check so that `/cmd @file` - // triggers file completion, not just slash command completion. - for (let i = cursorCol - 1; i >= 0; i--) { - const char = codePoints[i]; + if (shellModeActive) { + const tokenInfo = getTokenAtCursor(currentLine, cursorCol); + if (tokenInfo) { + return { + completionMode: CompletionMode.SHELL, + query: tokenInfo.token, + completionStart: tokenInfo.start, + completionEnd: tokenInfo.end, + shellTokenIsCommand: tokenInfo.isFirstToken, + shellTokens: tokenInfo.tokens, + shellCursorIndex: tokenInfo.cursorIndex, + shellCommandToken: tokenInfo.commandToken, + }; + } + return { + completionMode: CompletionMode.SHELL, + query: '', + completionStart: cursorCol, + completionEnd: cursorCol, + shellTokenIsCommand: currentLine.trim().length === 0, + shellTokens: [''], + shellCursorIndex: 0, + shellCommandToken: '', + }; + } - if (char === ' ') { - let backslashCount = 0; - for (let j = i - 1; j >= 0 && codePoints[j] === '\\'; j--) { - backslashCount++; - } - if (backslashCount % 2 === 0) { - break; - } - } else if (char === '@') { - let end = codePoints.length; - for (let i = cursorCol; i < codePoints.length; i++) { - if (codePoints[i] === ' ') { - let backslashCount = 0; - for (let j = i - 1; j >= 0 && codePoints[j] === '\\'; j--) { - backslashCount++; - } + // FIRST: Check for @ completion (scan backwards from cursor) + // This must happen before slash command check so that `/cmd @file` + // triggers file completion, not just slash command completion. + for (let i = cursorCol - 1; i >= 0; i--) { + const char = codePoints[i]; - if (backslashCount % 2 === 0) { - end = i; - break; - } + if (char === ' ') { + let backslashCount = 0; + for (let j = i - 1; j >= 0 && codePoints[j] === '\\'; j--) { + backslashCount++; + } + if (backslashCount % 2 === 0) { + break; + } + } else if (char === '@') { + let end = codePoints.length; + for (let i = cursorCol; i < codePoints.length; i++) { + if (codePoints[i] === ' ') { + let backslashCount = 0; + for (let j = i - 1; j >= 0 && codePoints[j] === '\\'; j--) { + backslashCount++; + } + + if (backslashCount % 2 === 0) { + end = i; + break; } } - const pathStart = i + 1; - const partialPath = currentLine.substring(pathStart, end); - return { - completionMode: CompletionMode.AT, - query: partialPath, - completionStart: pathStart, - completionEnd: end, - }; } - } - - // THEN: Check for slash command (only if no @ completion is active) - if (cursorRow === 0 && isSlashCommand(currentLine.trim())) { + const pathStart = i + 1; + const partialPath = currentLine.substring(pathStart, end); return { - completionMode: CompletionMode.SLASH, - query: currentLine, - completionStart: 0, - completionEnd: currentLine.length, - }; - } - - // Check for prompt completion - only if enabled - const trimmedText = buffer.text.trim(); - const isPromptCompletionEnabled = false; - if ( - isPromptCompletionEnabled && - trimmedText.length >= PROMPT_COMPLETION_MIN_LENGTH && - !isSlashCommand(trimmedText) && - !trimmedText.includes('@') - ) { - return { - completionMode: CompletionMode.PROMPT, - query: trimmedText, - completionStart: 0, - completionEnd: trimmedText.length, + completionMode: CompletionMode.AT, + query: partialPath, + completionStart: pathStart, + completionEnd: end, + shellTokenIsCommand: false, + shellTokens: [], + shellCursorIndex: -1, + shellCommandToken: '', }; } + } + // THEN: Check for slash command (only if no @ completion is active) + if (cursorRow === 0 && isSlashCommand(currentLine.trim())) { return { - completionMode: CompletionMode.IDLE, - query: null, - completionStart: -1, - completionEnd: -1, + completionMode: CompletionMode.SLASH, + query: currentLine, + completionStart: 0, + completionEnd: currentLine.length, + shellTokenIsCommand: false, + shellTokens: [], + shellCursorIndex: -1, + shellCommandToken: '', }; - }, [cursorRow, cursorCol, buffer.lines, buffer.text]); + } + + // Check for prompt completion - only if enabled + const trimmedText = buffer.text.trim(); + const isPromptCompletionEnabled = false; + if ( + isPromptCompletionEnabled && + trimmedText.length >= PROMPT_COMPLETION_MIN_LENGTH && + !isSlashCommand(trimmedText) && + !trimmedText.includes('@') + ) { + return { + completionMode: CompletionMode.PROMPT, + query: trimmedText, + completionStart: 0, + completionEnd: trimmedText.length, + shellTokenIsCommand: false, + shellTokens: [], + shellCursorIndex: -1, + shellCommandToken: '', + }; + } + + return { + completionMode: CompletionMode.IDLE, + query: null, + completionStart: -1, + completionEnd: -1, + shellTokenIsCommand: false, + shellTokens: [], + shellCursorIndex: -1, + shellCommandToken: '', + }; + }, [cursorRow, cursorCol, buffer.lines, buffer.text, shellModeActive]); useAtCompletion({ enabled: active && completionMode === CompletionMode.AT, @@ -199,9 +251,20 @@ export function useCommandCompletion({ setIsPerfectMatch, }); + useShellCompletion({ + enabled: active && completionMode === CompletionMode.SHELL, + query: query || '', + isCommandPosition: shellTokenIsCommand, + tokens: shellTokens, + cursorIndex: shellCursorIndex, + commandToken: shellCommandToken, + cwd, + setSuggestions, + setIsLoadingSuggestions, + }); + const promptCompletion = usePromptCompletion({ buffer, - config, }); useEffect(() => { diff --git a/packages/cli/src/ui/hooks/useShellCompletion.test.ts b/packages/cli/src/ui/hooks/useShellCompletion.test.ts new file mode 100644 index 0000000000..477db3b184 --- /dev/null +++ b/packages/cli/src/ui/hooks/useShellCompletion.test.ts @@ -0,0 +1,405 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { + getTokenAtCursor, + escapeShellPath, + resolvePathCompletions, + scanPathExecutables, +} from './useShellCompletion.js'; +import type { FileSystemStructure } from '@google/gemini-cli-test-utils'; +import { createTmpDir, cleanupTmpDir } from '@google/gemini-cli-test-utils'; + +describe('useShellCompletion utilities', () => { + describe('getTokenAtCursor', () => { + it('should return empty token struct for empty line', () => { + expect(getTokenAtCursor('', 0)).toEqual({ + token: '', + start: 0, + end: 0, + isFirstToken: true, + tokens: [''], + cursorIndex: 0, + commandToken: '', + }); + }); + + it('should extract the first token at cursor position 0', () => { + const result = getTokenAtCursor('git status', 3); + expect(result).toEqual({ + token: 'git', + start: 0, + end: 3, + isFirstToken: true, + tokens: ['git', 'status'], + cursorIndex: 0, + commandToken: 'git', + }); + }); + + it('should extract the second token when cursor is on it', () => { + const result = getTokenAtCursor('git status', 7); + expect(result).toEqual({ + token: 'status', + start: 4, + end: 10, + isFirstToken: false, + tokens: ['git', 'status'], + cursorIndex: 1, + commandToken: 'git', + }); + }); + + it('should handle cursor at start of second token', () => { + const result = getTokenAtCursor('git status', 4); + expect(result).toEqual({ + token: 'status', + start: 4, + end: 10, + isFirstToken: false, + tokens: ['git', 'status'], + cursorIndex: 1, + commandToken: 'git', + }); + }); + + it('should handle escaped spaces', () => { + const result = getTokenAtCursor('cat my\\ file.txt', 16); + expect(result).toEqual({ + token: 'my file.txt', + start: 4, + end: 16, + isFirstToken: false, + tokens: ['cat', 'my file.txt'], + cursorIndex: 1, + commandToken: 'cat', + }); + }); + + it('should handle single-quoted strings', () => { + const result = getTokenAtCursor("cat 'my file.txt'", 17); + expect(result).toEqual({ + token: 'my file.txt', + start: 4, + end: 17, + isFirstToken: false, + tokens: ['cat', 'my file.txt'], + cursorIndex: 1, + commandToken: 'cat', + }); + }); + + it('should handle double-quoted strings', () => { + const result = getTokenAtCursor('cat "my file.txt"', 17); + expect(result).toEqual({ + token: 'my file.txt', + start: 4, + end: 17, + isFirstToken: false, + tokens: ['cat', 'my file.txt'], + cursorIndex: 1, + commandToken: 'cat', + }); + }); + + it('should handle cursor past all tokens (trailing space)', () => { + const result = getTokenAtCursor('git ', 4); + expect(result).toEqual({ + token: '', + start: 4, + end: 4, + isFirstToken: false, + tokens: ['git', ''], + cursorIndex: 1, + commandToken: 'git', + }); + }); + + it('should handle cursor in the middle of a word', () => { + const result = getTokenAtCursor('git checkout main', 7); + expect(result).toEqual({ + token: 'checkout', + start: 4, + end: 12, + isFirstToken: false, + tokens: ['git', 'checkout', 'main'], + cursorIndex: 1, + commandToken: 'git', + }); + }); + + it('should mark isFirstToken correctly for first word', () => { + const result = getTokenAtCursor('gi', 2); + expect(result?.isFirstToken).toBe(true); + }); + + it('should mark isFirstToken correctly for second word', () => { + const result = getTokenAtCursor('git sta', 7); + expect(result?.isFirstToken).toBe(false); + }); + + it('should handle cursor in whitespace between tokens', () => { + const result = getTokenAtCursor('git status', 4); + expect(result).toEqual({ + token: '', + start: 4, + end: 4, + isFirstToken: false, + tokens: ['git', '', 'status'], + cursorIndex: 1, + commandToken: 'git', + }); + }); + }); + + describe('escapeShellPath', () => { + const isWin = process.platform === 'win32'; + + it('should escape spaces', () => { + expect(escapeShellPath('my file.txt')).toBe( + isWin ? 'my file.txt' : 'my\\ file.txt', + ); + }); + + it('should escape parentheses', () => { + expect(escapeShellPath('file (copy).txt')).toBe( + isWin ? 'file (copy).txt' : 'file\\ \\(copy\\).txt', + ); + }); + + it('should not escape normal characters', () => { + expect(escapeShellPath('normal-file.txt')).toBe('normal-file.txt'); + }); + + it('should escape tabs, newlines, carriage returns, and backslashes', () => { + if (isWin) { + expect(escapeShellPath('a\tb')).toBe('a\tb'); + expect(escapeShellPath('a\nb')).toBe('a\nb'); + expect(escapeShellPath('a\rb')).toBe('a\rb'); + expect(escapeShellPath('a\\b')).toBe('a\\b'); + } else { + expect(escapeShellPath('a\tb')).toBe('a\\\tb'); + expect(escapeShellPath('a\nb')).toBe('a\\\nb'); + expect(escapeShellPath('a\rb')).toBe('a\\\rb'); + expect(escapeShellPath('a\\b')).toBe('a\\\\b'); + } + }); + + it('should handle empty string', () => { + expect(escapeShellPath('')).toBe(''); + }); + }); + + describe('resolvePathCompletions', () => { + let tmpDir: string; + + afterEach(async () => { + if (tmpDir) { + await cleanupTmpDir(tmpDir); + } + }); + + it('should list directory contents for empty partial', async () => { + const structure: FileSystemStructure = { + 'file.txt': '', + subdir: {}, + }; + tmpDir = await createTmpDir(structure); + + const results = await resolvePathCompletions('', tmpDir); + const values = results.map((s) => s.label); + expect(values).toContain('subdir/'); + expect(values).toContain('file.txt'); + }); + + it('should filter by prefix', async () => { + const structure: FileSystemStructure = { + 'abc.txt': '', + 'def.txt': '', + }; + tmpDir = await createTmpDir(structure); + + const results = await resolvePathCompletions('a', tmpDir); + expect(results).toHaveLength(1); + expect(results[0].label).toBe('abc.txt'); + }); + + it('should match case-insensitively', async () => { + const structure: FileSystemStructure = { + Desktop: {}, + }; + tmpDir = await createTmpDir(structure); + + const results = await resolvePathCompletions('desk', tmpDir); + expect(results).toHaveLength(1); + expect(results[0].label).toBe('Desktop/'); + }); + + it('should append trailing slash to directories', async () => { + const structure: FileSystemStructure = { + mydir: {}, + 'myfile.txt': '', + }; + tmpDir = await createTmpDir(structure); + + const results = await resolvePathCompletions('my', tmpDir); + const dirSuggestion = results.find((s) => s.label.startsWith('mydir')); + expect(dirSuggestion?.label).toBe('mydir/'); + expect(dirSuggestion?.description).toBe('directory'); + }); + + it('should hide dotfiles by default', async () => { + const structure: FileSystemStructure = { + '.hidden': '', + visible: '', + }; + tmpDir = await createTmpDir(structure); + + const results = await resolvePathCompletions('', tmpDir); + const labels = results.map((s) => s.label); + expect(labels).not.toContain('.hidden'); + expect(labels).toContain('visible'); + }); + + it('should show dotfiles when query starts with a dot', async () => { + const structure: FileSystemStructure = { + '.hidden': '', + '.bashrc': '', + visible: '', + }; + tmpDir = await createTmpDir(structure); + + const results = await resolvePathCompletions('.h', tmpDir); + const labels = results.map((s) => s.label); + expect(labels).toContain('.hidden'); + }); + + it('should show dotfiles in the current directory when query is exactly "."', async () => { + const structure: FileSystemStructure = { + '.hidden': '', + '.bashrc': '', + visible: '', + }; + tmpDir = await createTmpDir(structure); + + const results = await resolvePathCompletions('.', tmpDir); + const labels = results.map((s) => s.label); + expect(labels).toContain('.hidden'); + expect(labels).toContain('.bashrc'); + expect(labels).not.toContain('visible'); + }); + + it('should handle dotfile completions within a subdirectory', async () => { + const structure: FileSystemStructure = { + subdir: { + '.secret': '', + 'public.txt': '', + }, + }; + tmpDir = await createTmpDir(structure); + + const results = await resolvePathCompletions('subdir/.', tmpDir); + const labels = results.map((s) => s.label); + expect(labels).toContain('.secret'); + expect(labels).not.toContain('public.txt'); + }); + + it('should strip leading quotes to resolve inner directory contents', async () => { + const structure: FileSystemStructure = { + src: { + 'index.ts': '', + }, + }; + tmpDir = await createTmpDir(structure); + + const results = await resolvePathCompletions('"src/', tmpDir); + expect(results).toHaveLength(1); + expect(results[0].label).toBe('index.ts'); + + const resultsSingleQuote = await resolvePathCompletions("'src/", tmpDir); + expect(resultsSingleQuote).toHaveLength(1); + expect(resultsSingleQuote[0].label).toBe('index.ts'); + }); + + it('should properly escape resolutions with spaces inside stripped quote queries', async () => { + const structure: FileSystemStructure = { + 'Folder With Spaces': {}, + }; + tmpDir = await createTmpDir(structure); + + const results = await resolvePathCompletions('"Fo', tmpDir); + expect(results).toHaveLength(1); + expect(results[0].label).toBe('Folder With Spaces/'); + expect(results[0].value).toBe(escapeShellPath('Folder With Spaces/')); + }); + + it('should return empty array for non-existent directory', async () => { + const results = await resolvePathCompletions( + '/nonexistent/path/foo', + '/tmp', + ); + expect(results).toEqual([]); + }); + + it('should handle tilde expansion', async () => { + // Just ensure ~ doesn't throw + const results = await resolvePathCompletions('~/', '/tmp'); + // We can't assert specific files since it depends on the test runner's home + expect(Array.isArray(results)).toBe(true); + }); + + it('should escape special characters in results', async () => { + const isWin = process.platform === 'win32'; + const structure: FileSystemStructure = { + 'my file.txt': '', + }; + tmpDir = await createTmpDir(structure); + + const results = await resolvePathCompletions('my', tmpDir); + expect(results).toHaveLength(1); + expect(results[0].value).toBe(isWin ? 'my file.txt' : 'my\\ file.txt'); + }); + + it('should sort directories before files', async () => { + const structure: FileSystemStructure = { + 'b-file.txt': '', + 'a-dir': {}, + }; + tmpDir = await createTmpDir(structure); + + const results = await resolvePathCompletions('', tmpDir); + expect(results[0].description).toBe('directory'); + expect(results[1].description).toBe('file'); + }); + }); + + describe('scanPathExecutables', () => { + it('should return an array of executables', async () => { + const results = await scanPathExecutables(); + expect(Array.isArray(results)).toBe(true); + // Very basic sanity check: common commands should be found + if (process.platform !== 'win32') { + expect(results).toContain('ls'); + } + }); + + it('should support abort signal', async () => { + const controller = new AbortController(); + controller.abort(); + const results = await scanPathExecutables(controller.signal); + // May return empty or partial depending on timing + expect(Array.isArray(results)).toBe(true); + }); + + it('should handle empty PATH', async () => { + vi.stubEnv('PATH', ''); + const results = await scanPathExecutables(); + expect(results).toEqual([]); + vi.unstubAllEnvs(); + }); + }); +}); diff --git a/packages/cli/src/ui/hooks/useShellCompletion.ts b/packages/cli/src/ui/hooks/useShellCompletion.ts new file mode 100644 index 0000000000..8569ab5cfb --- /dev/null +++ b/packages/cli/src/ui/hooks/useShellCompletion.ts @@ -0,0 +1,548 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { useEffect, useRef, useCallback } from 'react'; +import * as fs from 'node:fs/promises'; +import * as path from 'node:path'; +import * as os from 'node:os'; +import type { Suggestion } from '../components/SuggestionsDisplay.js'; +import { debugLogger } from '@google/gemini-cli-core'; +import { getArgumentCompletions } from './shell-completions/index.js'; + +/** + * Maximum number of suggestions to return to avoid freezing the React Ink UI. + */ +const MAX_SHELL_SUGGESTIONS = 100; + +/** + * Debounce interval (ms) for file system completions. + */ +const FS_COMPLETION_DEBOUNCE_MS = 50; + +// Backslash-quote shell metacharacters on non-Windows platforms. + +// On Unix, backslash-quote shell metacharacters (spaces, parens, etc.). +// On Windows, cmd.exe doesn't use backslash-quoting and `\` is the path +// separator, so we leave the path as-is. +const UNIX_SHELL_SPECIAL_CHARS = /[ \t\n\r'"()&|;<>!#$`{}[\]*?\\]/g; + +/** + * Escapes special shell characters in a path segment. + */ +export function escapeShellPath(segment: string): string { + if (process.platform === 'win32') { + return segment; + } + return segment.replace(UNIX_SHELL_SPECIAL_CHARS, '\\$&'); +} + +export interface TokenInfo { + /** The raw token text (without surrounding quotes but with internal escapes). */ + token: string; + /** Offset in the original line where this token begins. */ + start: number; + /** Offset in the original line where this token ends (exclusive). */ + end: number; + /** Whether this is the first token (command position). */ + isFirstToken: boolean; + /** The fully built list of tokens parsing the string. */ + tokens: string[]; + /** The index in the tokens list where the cursor lies. */ + cursorIndex: number; + /** The command token (always tokens[0] if length > 0, otherwise empty string) */ + commandToken: string; +} + +export function getTokenAtCursor( + line: string, + cursorCol: number, +): TokenInfo | null { + const tokensInfo: Array<{ token: string; start: number; end: number }> = []; + let i = 0; + + while (i < line.length) { + // Skip whitespace + if (line[i] === ' ' || line[i] === '\t') { + i++; + continue; + } + + const tokenStart = i; + let token = ''; + + while (i < line.length) { + const ch = line[i]; + + // Backslash escape: consume the next char literally + if (ch === '\\' && i + 1 < line.length) { + token += line[i + 1]; + i += 2; + continue; + } + + // Single-quoted string + if (ch === "'") { + i++; // skip opening quote + while (i < line.length && line[i] !== "'") { + token += line[i]; + i++; + } + if (i < line.length) i++; // skip closing quote + continue; + } + + // Double-quoted string + if (ch === '"') { + i++; // skip opening quote + while (i < line.length && line[i] !== '"') { + if (line[i] === '\\' && i + 1 < line.length) { + token += line[i + 1]; + i += 2; + } else { + token += line[i]; + i++; + } + } + if (i < line.length) i++; // skip closing quote + continue; + } + + // Unquoted whitespace ends the token + if (ch === ' ' || ch === '\t') { + break; + } + + token += ch; + i++; + } + + tokensInfo.push({ token, start: tokenStart, end: i }); + } + + const rawTokens = tokensInfo.map((t) => t.token); + const commandToken = rawTokens.length > 0 ? rawTokens[0] : ''; + + if (tokensInfo.length === 0) { + return { + token: '', + start: cursorCol, + end: cursorCol, + isFirstToken: true, + tokens: [''], + cursorIndex: 0, + commandToken: '', + }; + } + + // Find the token that contains or is immediately adjacent to the cursor + for (let idx = 0; idx < tokensInfo.length; idx++) { + const t = tokensInfo[idx]; + if (cursorCol >= t.start && cursorCol <= t.end) { + return { + token: t.token, + start: t.start, + end: t.end, + isFirstToken: idx === 0, + tokens: rawTokens, + cursorIndex: idx, + commandToken, + }; + } + } + + // Cursor is in whitespace between tokens, or at the start/end of the line. + // Find the appropriate insertion index for a new empty token. + let insertIndex = tokensInfo.length; + for (let idx = 0; idx < tokensInfo.length; idx++) { + if (cursorCol < tokensInfo[idx].start) { + insertIndex = idx; + break; + } + } + + const newTokens = [ + ...rawTokens.slice(0, insertIndex), + '', + ...rawTokens.slice(insertIndex), + ]; + + return { + token: '', + start: cursorCol, + end: cursorCol, + isFirstToken: insertIndex === 0, + tokens: newTokens, + cursorIndex: insertIndex, + commandToken: newTokens.length > 0 ? newTokens[0] : '', + }; +} + +export async function scanPathExecutables( + signal?: AbortSignal, +): Promise { + const pathEnv = process.env['PATH'] ?? ''; + const dirs = pathEnv.split(path.delimiter).filter(Boolean); + const isWindows = process.platform === 'win32'; + const pathExtList = isWindows + ? (process.env['PATHEXT'] ?? '.EXE;.CMD;.BAT;.COM') + .split(';') + .filter(Boolean) + .map((e) => e.toLowerCase()) + : []; + + const seen = new Set(); + const executables: string[] = []; + + const dirResults = await Promise.all( + dirs.map(async (dir) => { + if (signal?.aborted) return []; + try { + const entries = await fs.readdir(dir, { withFileTypes: true }); + const validEntries: string[] = []; + + // Check executability in parallel (batched per directory) + await Promise.all( + entries.map(async (entry) => { + if (signal?.aborted) return; + if (!entry.isFile() && !entry.isSymbolicLink()) return; + + const name = entry.name; + if (isWindows) { + const ext = path.extname(name).toLowerCase(); + if (pathExtList.length > 0 && !pathExtList.includes(ext)) return; + } + + try { + await fs.access( + path.join(dir, name), + fs.constants.R_OK | fs.constants.X_OK, + ); + validEntries.push(name); + } catch { + // Not executable — skip + } + }), + ); + + return validEntries; + } catch { + // EACCES, ENOENT, etc. — skip this directory + return []; + } + }), + ); + + for (const names of dirResults) { + for (const name of names) { + if (!seen.has(name)) { + seen.add(name); + executables.push(name); + } + } + } + + executables.sort(); + return executables; +} + +function expandTilde(inputPath: string): [string, boolean] { + if ( + inputPath === '~' || + inputPath.startsWith('~/') || + inputPath.startsWith('~' + path.sep) + ) { + return [path.join(os.homedir(), inputPath.slice(1)), true]; + } + return [inputPath, false]; +} + +export async function resolvePathCompletions( + partial: string, + cwd: string, + signal?: AbortSignal, +): Promise { + if (partial == null) return []; + + // Input Sanitization + let strippedPartial = partial; + if (strippedPartial.startsWith('"') || strippedPartial.startsWith("'")) { + strippedPartial = strippedPartial.slice(1); + } + if (strippedPartial.endsWith('"') || strippedPartial.endsWith("'")) { + strippedPartial = strippedPartial.slice(0, -1); + } + + // Normalize separators \ to / + const normalizedPartial = strippedPartial.replace(/\\/g, '/'); + + const [expandedPartial, didExpandTilde] = expandTilde(normalizedPartial); + + // Directory Detection + const endsWithSep = + normalizedPartial.endsWith('/') || normalizedPartial === ''; + const dirToRead = endsWithSep + ? path.resolve(cwd, expandedPartial) + : path.resolve(cwd, path.dirname(expandedPartial)); + + const prefix = endsWithSep ? '' : path.basename(expandedPartial); + const prefixLower = prefix.toLowerCase(); + + const showDotfiles = prefix.startsWith('.'); + + let entries: Array; + try { + if (signal?.aborted) return []; + entries = await fs.readdir(dirToRead, { withFileTypes: true }); + } catch { + // EACCES, ENOENT, etc. + return []; + } + + if (signal?.aborted) return []; + + const suggestions: Suggestion[] = []; + for (const entry of entries) { + if (signal?.aborted) break; + + const name = entry.name; + + // Hide dotfiles unless query starts with '.' + if (name.startsWith('.') && !showDotfiles) continue; + + // Case-insensitive matching + if (!name.toLowerCase().startsWith(prefixLower)) continue; + + const isDir = entry.isDirectory(); + const displayName = isDir ? name + '/' : name; + + // Build the completion value relative to what the user typed + let completionValue: string; + if (endsWithSep) { + completionValue = normalizedPartial + displayName; + } else { + const parentPart = normalizedPartial.slice( + 0, + normalizedPartial.length - path.basename(normalizedPartial).length, + ); + completionValue = parentPart + displayName; + } + + // Restore tilde if we expanded it + if (didExpandTilde) { + const homeDir = os.homedir().replace(/\\/g, '/'); + if (completionValue.startsWith(homeDir)) { + completionValue = '~' + completionValue.slice(homeDir.length); + } + } + + // Output formatting: Escape special characters in the completion value + // Since normalizedPartial stripped quotes, we escape the value directly. + const escapedValue = escapeShellPath(completionValue); + + suggestions.push({ + label: displayName, + value: escapedValue, + description: isDir ? 'directory' : 'file', + }); + + if (suggestions.length >= MAX_SHELL_SUGGESTIONS) break; + } + + // Sort: directories first, then alphabetically + suggestions.sort((a, b) => { + const aIsDir = a.description === 'directory'; + const bIsDir = b.description === 'directory'; + if (aIsDir !== bIsDir) return aIsDir ? -1 : 1; + return a.label.localeCompare(b.label); + }); + + return suggestions; +} + +export interface UseShellCompletionProps { + /** Whether shell completion is active. */ + enabled: boolean; + /** The partial query string (the token under the cursor). */ + query: string; + /** Whether the token is in command position (first word). */ + isCommandPosition: boolean; + /** The full list of parsed tokens */ + tokens: string[]; + /** The cursor index in the full list of parsed tokens */ + cursorIndex: number; + /** The root command token */ + commandToken: string; + /** The current working directory for path resolution. */ + cwd: string; + /** Callback to set suggestions on the parent state. */ + setSuggestions: (suggestions: Suggestion[]) => void; + /** Callback to set loading state on the parent. */ + setIsLoadingSuggestions: (isLoading: boolean) => void; +} + +export function useShellCompletion({ + enabled, + query, + isCommandPosition, + tokens, + cursorIndex, + commandToken, + cwd, + setSuggestions, + setIsLoadingSuggestions, +}: UseShellCompletionProps): void { + const pathCacheRef = useRef(null); + const pathEnvRef = useRef(process.env['PATH'] ?? ''); + const abortRef = useRef(null); + const debounceRef = useRef(null); + + // Invalidate PATH cache when $PATH changes + useEffect(() => { + const currentPath = process.env['PATH'] ?? ''; + if (currentPath !== pathEnvRef.current) { + pathCacheRef.current = null; + pathEnvRef.current = currentPath; + } + }); + + const performCompletion = useCallback(async () => { + if (!enabled) { + setSuggestions([]); + return; + } + + // Skip flags + if (query.startsWith('-')) { + setSuggestions([]); + return; + } + + // Cancel any in-flight request + if (abortRef.current) { + abortRef.current.abort(); + } + const controller = new AbortController(); + abortRef.current = controller; + const { signal } = controller; + + try { + let results: Suggestion[]; + + if (isCommandPosition) { + setIsLoadingSuggestions(true); + + if (!pathCacheRef.current) { + pathCacheRef.current = await scanPathExecutables(signal); + } + + if (signal.aborted) return; + + const queryLower = query.toLowerCase(); + results = pathCacheRef.current + .filter((cmd) => cmd.toLowerCase().startsWith(queryLower)) + .slice(0, MAX_SHELL_SUGGESTIONS) + .map((cmd) => ({ + label: cmd, + value: escapeShellPath(cmd), + description: 'command', + })); + } else { + const argumentCompletions = await getArgumentCompletions( + commandToken, + tokens, + cursorIndex, + cwd, + signal, + ); + + if (signal.aborted) return; + + if (argumentCompletions?.exclusive) { + results = argumentCompletions.suggestions; + } else { + const pathSuggestions = await resolvePathCompletions( + query, + cwd, + signal, + ); + if (signal.aborted) return; + + results = [ + ...(argumentCompletions?.suggestions ?? []), + ...pathSuggestions, + ].slice(0, MAX_SHELL_SUGGESTIONS); + } + } + + if (signal.aborted) return; + + setSuggestions(results); + } catch (error) { + if ( + !( + signal.aborted || + (error instanceof Error && error.name === 'AbortError') + ) + ) { + debugLogger.warn( + `[WARN] shell completion failed: ${error instanceof Error ? error.message : String(error)}`, + ); + } + if (!signal.aborted) { + setSuggestions([]); + } + } finally { + if (!signal.aborted) { + setIsLoadingSuggestions(false); + } + } + }, [ + enabled, + query, + isCommandPosition, + tokens, + cursorIndex, + commandToken, + cwd, + setSuggestions, + setIsLoadingSuggestions, + ]); + + // Debounced effect to trigger completion + useEffect(() => { + if (!enabled) { + setSuggestions([]); + setIsLoadingSuggestions(false); + return; + } + + if (debounceRef.current) { + clearTimeout(debounceRef.current); + } + + debounceRef.current = setTimeout(() => { + // eslint-disable-next-line @typescript-eslint/no-floating-promises + performCompletion(); + }, FS_COMPLETION_DEBOUNCE_MS); + + return () => { + if (debounceRef.current) { + clearTimeout(debounceRef.current); + } + }; + }, [enabled, performCompletion, setSuggestions, setIsLoadingSuggestions]); + + // Cleanup on unmount + useEffect( + () => () => { + abortRef.current?.abort(); + if (debounceRef.current) { + clearTimeout(debounceRef.current); + } + }, + [], + ); +} diff --git a/packages/cli/src/ui/utils/editorUtils.ts b/packages/cli/src/ui/utils/editorUtils.ts new file mode 100644 index 0000000000..7b9efd5a81 --- /dev/null +++ b/packages/cli/src/ui/utils/editorUtils.ts @@ -0,0 +1,151 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { spawn, spawnSync } from 'node:child_process'; +import type { ReadStream } from 'node:tty'; +import { + coreEvents, + CoreEvent, + type EditorType, + getEditorCommand, + isGuiEditor, + isTerminalEditor, +} from '@google/gemini-cli-core'; + +/** + * Opens a file in an external editor and waits for it to close. + * Handles raw mode switching to ensure the editor can interact with the terminal. + * + * @param filePath Path to the file to open + * @param stdin The stdin stream from Ink/Node + * @param setRawMode Function to toggle raw mode + * @param preferredEditorType The user's preferred editor from config + */ +export async function openFileInEditor( + filePath: string, + stdin: ReadStream | null | undefined, + setRawMode: ((mode: boolean) => void) | undefined, + preferredEditorType?: EditorType, +): Promise { + let command: string | undefined = undefined; + const args = [filePath]; + + if (preferredEditorType) { + command = getEditorCommand(preferredEditorType); + if (isGuiEditor(preferredEditorType)) { + args.unshift('--wait'); + } + } + + if (!command) { + command = process.env['VISUAL'] ?? process.env['EDITOR']; + if (command) { + const lowerCommand = command.toLowerCase(); + const isGui = ['code', 'cursor', 'subl', 'zed', 'atom'].some((gui) => + lowerCommand.includes(gui), + ); + if ( + isGui && + !lowerCommand.includes('--wait') && + !lowerCommand.includes('-w') + ) { + args.unshift(lowerCommand.includes('subl') ? '-w' : '--wait'); + } + } + } + + if (!command) { + command = process.platform === 'win32' ? 'notepad' : 'vi'; + } + + const [executable = '', ...initialArgs] = command.split(' '); + + // Determine if we should use sync or async based on the command/editor type. + // If we have a preferredEditorType, we can check if it's a terminal editor. + // Otherwise, we guess based on the command name. + const terminalEditors = ['vi', 'vim', 'nvim', 'emacs', 'hx', 'nano']; + const isTerminal = preferredEditorType + ? isTerminalEditor(preferredEditorType) + : terminalEditors.some((te) => executable.toLowerCase().includes(te)); + + if ( + isTerminal && + (executable.includes('vi') || + executable.includes('vim') || + executable.includes('nvim')) + ) { + // Pass -i NONE to prevent E138 'Can't write viminfo file' errors in restricted environments. + args.unshift('-i', 'NONE'); + } + + const wasRaw = stdin?.isRaw ?? false; + setRawMode?.(false); + + try { + if (isTerminal) { + const result = spawnSync(executable, [...initialArgs, ...args], { + stdio: 'inherit', + shell: process.platform === 'win32', + }); + if (result.error) { + coreEvents.emitFeedback( + 'error', + '[editorUtils] external terminal editor error', + result.error, + ); + throw result.error; + } + if (typeof result.status === 'number' && result.status !== 0) { + const err = new Error( + `External editor exited with status ${result.status}`, + ); + coreEvents.emitFeedback( + 'error', + '[editorUtils] external editor error', + err, + ); + throw err; + } + } else { + await new Promise((resolve, reject) => { + const child = spawn(executable, [...initialArgs, ...args], { + stdio: 'inherit', + shell: process.platform === 'win32', + }); + + child.on('error', (err) => { + coreEvents.emitFeedback( + 'error', + '[editorUtils] external editor spawn error', + err, + ); + reject(err); + }); + + child.on('close', (status) => { + if (typeof status === 'number' && status !== 0) { + const err = new Error( + `External editor exited with status ${status}`, + ); + coreEvents.emitFeedback( + 'error', + '[editorUtils] external editor error', + err, + ); + reject(err); + } else { + resolve(); + } + }); + }); + } + } finally { + if (wasRaw) { + setRawMode?.(true); + } + coreEvents.emit(CoreEvent.ExternalEditorClosed); + } +} diff --git a/packages/core/src/policy/policies/plan.toml b/packages/core/src/policy/policies/plan.toml index 3a26fab679..a490e589b0 100644 --- a/packages/core/src/policy/policies/plan.toml +++ b/packages/core/src/policy/policies/plan.toml @@ -50,7 +50,7 @@ priority = 70 modes = ["plan"] [[rule]] -toolName = ["ask_user", "exit_plan_mode"] +toolName = ["ask_user", "exit_plan_mode", "save_memory"] decision = "ask_user" 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 0d110f8b2d..7accf5c7e5 100644 --- a/packages/core/src/policy/policy-engine.test.ts +++ b/packages/core/src/policy/policy-engine.test.ts @@ -2601,6 +2601,12 @@ describe('PolicyEngine', () => { priority: 70, modes: [ApprovalMode.PLAN], }, + { + toolName: 'save_memory', + decision: PolicyDecision.ASK_USER, + priority: 70, + modes: [ApprovalMode.PLAN], + }, { toolName: 'exit_plan_mode', decision: PolicyDecision.ASK_USER, @@ -2638,6 +2644,7 @@ describe('PolicyEngine', () => { 'web_fetch', 'write_todos', 'memory', + 'save_memory', 'read_tool', 'write_tool', ]); @@ -2667,6 +2674,7 @@ describe('PolicyEngine', () => { expect(excluded.has('activate_skill')).toBe(false); expect(excluded.has('ask_user')).toBe(false); expect(excluded.has('exit_plan_mode')).toBe(false); + expect(excluded.has('save_memory')).toBe(false); // Read-only MCP tool allowed by annotation rule (matched via _serverName) expect(excluded.has('read_tool')).toBe(false); }); diff --git a/packages/core/src/utils/flashFallback.test.ts b/packages/core/src/utils/flashFallback.test.ts index ec95de94ef..af4a73c213 100644 --- a/packages/core/src/utils/flashFallback.test.ts +++ b/packages/core/src/utils/flashFallback.test.ts @@ -19,6 +19,7 @@ import { AuthType } from '../core/contentGenerator.js'; // Import the new types (Assuming this test file is in packages/core/src/utils/) import type { FallbackModelHandler } from '../fallback/types.js'; import type { GoogleApiError } from './googleErrors.js'; +import { type HttpError } from './httpErrors.js'; import { TerminalQuotaError } from './googleQuotaErrors.js'; vi.mock('node:fs'); @@ -106,6 +107,34 @@ describe('Retry Utility Fallback Integration', () => { expect(mockApiCall).toHaveBeenCalledTimes(3); }); + it('should trigger onPersistent429 when HTTP 499 persists through all retry attempts', async () => { + let fallbackCalled = false; + const mockError: HttpError = new Error('Simulated 499 error'); + mockError.status = 499; + + const mockApiCall = vi.fn().mockRejectedValue(mockError); // Always fail with 499 + + const mockPersistent429Callback = vi.fn(async (_authType?: string) => { + fallbackCalled = true; + // In a real scenario, this would change the model being called by mockApiCall + // or similar, but for the test we just need to see if it's called. + // We return null to stop retrying after the fallback attempt in this test. + return null; + }); + + const promise = retryWithBackoff(mockApiCall, { + maxAttempts: 2, + initialDelayMs: 1, + maxDelayMs: 10, + onPersistent429: mockPersistent429Callback, + authType: AuthType.LOGIN_WITH_GOOGLE, + }); + + await expect(promise).rejects.toThrow('Simulated 499 error'); + expect(fallbackCalled).toBe(true); + expect(mockPersistent429Callback).toHaveBeenCalledTimes(1); + }); + it('should not trigger onPersistent429 for API key users', async () => { const fallbackCallback = vi.fn(); diff --git a/packages/core/src/utils/googleQuotaErrors.test.ts b/packages/core/src/utils/googleQuotaErrors.test.ts index 06bde6444b..185f48e92a 100644 --- a/packages/core/src/utils/googleQuotaErrors.test.ts +++ b/packages/core/src/utils/googleQuotaErrors.test.ts @@ -81,7 +81,7 @@ describe('classifyGoogleError', () => { } }); - it('should return original error if code is not 429 or 503', () => { + it('should return original error if code is not 429, 499 or 503', () => { const apiError: GoogleApiError = { code: 500, message: 'Server error', @@ -95,6 +95,22 @@ describe('classifyGoogleError', () => { expect(result).not.toBeInstanceOf(RetryableQuotaError); }); + it('should return RetryableQuotaError for 499 Client Closed Request', () => { + const apiError: GoogleApiError = { + code: 499, + message: 'Client Closed Request', + details: [], + }; + vi.spyOn(errorParser, 'parseGoogleApiError').mockReturnValue(apiError); + const originalError = new Error('Client Closed Request'); + const result = classifyGoogleError(originalError); + expect(result).toBeInstanceOf(RetryableQuotaError); + if (result instanceof RetryableQuotaError) { + expect(result.cause).toBe(apiError); + expect(result.message).toBe('Client Closed Request'); + } + }); + it('should return TerminalQuotaError for daily quota violations in QuotaFailure', () => { const apiError: GoogleApiError = { code: 429, diff --git a/packages/core/src/utils/googleQuotaErrors.ts b/packages/core/src/utils/googleQuotaErrors.ts index 40c1c34361..a075b79b89 100644 --- a/packages/core/src/utils/googleQuotaErrors.ts +++ b/packages/core/src/utils/googleQuotaErrors.ts @@ -219,7 +219,7 @@ export function classifyGoogleError(error: unknown): unknown { if ( !googleApiError || - googleApiError.code !== 429 || + (googleApiError.code !== 429 && googleApiError.code !== 499) || googleApiError.details.length === 0 ) { // Fallback: try to parse the error message for a retry delay @@ -233,27 +233,27 @@ export function classifyGoogleError(error: unknown): unknown { return new RetryableQuotaError( errorMessage, googleApiError ?? { - code: 429, + code: status ?? 429, message: errorMessage, details: [], }, retryDelaySeconds, ); } - } else if (status === 429) { - // Fallback: If it is a 429 but doesn't have a specific "retry in" message, + } else if (status === 429 || status === 499) { + // Fallback: If it is a 429 or 499 but doesn't have a specific "retry in" message, // assume it is a temporary rate limit and retry after 5 sec (same as DEFAULT_RETRY_OPTIONS). return new RetryableQuotaError( errorMessage, googleApiError ?? { - code: 429, + code: status, message: errorMessage, details: [], }, ); } - return error; // Not a 429 error we can handle with structured details or a parsable retry message. + return error; // Not a retryable error we can handle with structured details or a parsable retry message. } const quotaFailure = googleApiError.details.find( @@ -353,15 +353,15 @@ export function classifyGoogleError(error: unknown): unknown { } } - // If we reached this point and the status is still 429, we return retryable. - if (status === 429) { + // If we reached this point and the status is still 429 or 499, we return retryable. + if (status === 429 || status === 499) { const errorMessage = googleApiError?.message || (error instanceof Error ? error.message : String(error)); return new RetryableQuotaError( errorMessage, googleApiError ?? { - code: 429, + code: status, message: errorMessage, details: [], }, diff --git a/packages/core/src/utils/retry.test.ts b/packages/core/src/utils/retry.test.ts index 43f038cfaa..f63a5ed723 100644 --- a/packages/core/src/utils/retry.test.ts +++ b/packages/core/src/utils/retry.test.ts @@ -158,6 +158,30 @@ describe('retryWithBackoff', () => { expect(mockFn).not.toHaveBeenCalled(); }); + it('should retry on HTTP 499 (Client Closed Request) error', async () => { + let attempts = 0; + const mockFn = vi.fn(async () => { + attempts++; + if (attempts === 1) { + const error: HttpError = new Error('Simulated 499 error'); + error.status = 499; + throw error; + } + return 'success'; + }); + + const promise = retryWithBackoff(mockFn, { + maxAttempts: 2, + initialDelayMs: 10, + }); + + await vi.runAllTimersAsync(); + + const result = await promise; + expect(result).toBe('success'); + expect(mockFn).toHaveBeenCalledTimes(2); + }); + it('should use default shouldRetry if not provided, retrying on ApiError 429', async () => { const mockFn = vi.fn(async () => { throw new ApiError({ message: 'Too Many Requests', status: 429 }); diff --git a/packages/core/src/utils/retry.ts b/packages/core/src/utils/retry.ts index 17c4a656ed..50c992d6de 100644 --- a/packages/core/src/utils/retry.ts +++ b/packages/core/src/utils/retry.ts @@ -130,13 +130,17 @@ export function isRetryableError( if (error instanceof ApiError) { // Explicitly do not retry 400 (Bad Request) if (error.status === 400) return false; - return error.status === 429 || (error.status >= 500 && error.status < 600); + return ( + error.status === 429 || + error.status === 499 || + (error.status >= 500 && error.status < 600) + ); } // Check for status using helper (handles other error shapes) const status = getErrorStatus(error); if (status !== undefined) { - return status === 429 || (status >= 500 && status < 600); + return status === 429 || status === 499 || (status >= 500 && status < 600); } return false; diff --git a/packages/sdk/src/agent.integration.test.ts b/packages/sdk/src/agent.integration.test.ts index 1de8e52ac7..78229a81cc 100644 --- a/packages/sdk/src/agent.integration.test.ts +++ b/packages/sdk/src/agent.integration.test.ts @@ -144,14 +144,14 @@ describe('GeminiCliAgent Integration', () => { }); it('propagates errors from dynamic instructions', async () => { + const goldenFile = getGoldenPath('agent-static-instructions'); const agent = new GeminiCliAgent({ instructions: () => { throw new Error('Dynamic instruction failure'); }, model: 'gemini-2.0-flash', - fakeResponses: RECORD_MODE - ? undefined - : getGoldenPath('agent-dynamic-instructions'), + recordResponses: RECORD_MODE ? goldenFile : undefined, + fakeResponses: RECORD_MODE ? undefined : goldenFile, }); const session = agent.session();