From 9e91aafe40591166002af1254a0f2a541c460512 Mon Sep 17 00:00:00 2001 From: Jacob Richman Date: Thu, 23 Oct 2025 09:35:32 -0700 Subject: [PATCH 01/12] Fix bug where tool scheduler was repeatedly created. (#11767) --- packages/cli/src/ui/AppContainer.tsx | 13 ++- .../ui/hooks/useReactToolScheduler.test.ts | 85 +++++++++++++++++++ .../cli/src/ui/hooks/useReactToolScheduler.ts | 40 +++++++-- packages/core/src/core/coreToolScheduler.ts | 9 +- 4 files changed, 131 insertions(+), 16 deletions(-) create mode 100644 packages/cli/src/ui/hooks/useReactToolScheduler.test.ts diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index 135516db6f..28ebc47d71 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -582,6 +582,15 @@ Logging in with Google... Please restart Gemini CLI to continue. const cancelHandlerRef = useRef<() => void>(() => {}); + const getPreferredEditor = useCallback( + () => settings.merged.general?.preferredEditor as EditorType, + [settings.merged.general?.preferredEditor], + ); + + const onCancelSubmit = useCallback(() => { + cancelHandlerRef.current(); + }, []); + const { streamingState, submitQuery, @@ -601,13 +610,13 @@ Logging in with Google... Please restart Gemini CLI to continue. setDebugMessage, handleSlashCommand, shellModeActive, - () => settings.merged.general?.preferredEditor as EditorType, + getPreferredEditor, onAuthError, performMemoryRefresh, modelSwitchedFromQuotaError, setModelSwitchedFromQuotaError, refreshStatic, - () => cancelHandlerRef.current(), + onCancelSubmit, setEmbeddedShellFocused, terminalWidth, terminalHeight, diff --git a/packages/cli/src/ui/hooks/useReactToolScheduler.test.ts b/packages/cli/src/ui/hooks/useReactToolScheduler.test.ts new file mode 100644 index 0000000000..b3fcfad8b7 --- /dev/null +++ b/packages/cli/src/ui/hooks/useReactToolScheduler.test.ts @@ -0,0 +1,85 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { CoreToolScheduler } from '@google/gemini-cli-core'; +import type { Config } from '@google/gemini-cli-core'; +import { renderHook } from '@testing-library/react'; +import { vi, describe, it, expect, beforeEach } from 'vitest'; +import { useReactToolScheduler } from './useReactToolScheduler.js'; + +vi.mock('@google/gemini-cli-core', async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + CoreToolScheduler: vi.fn(), + }; +}); + +const mockCoreToolScheduler = vi.mocked(CoreToolScheduler); + +describe('useReactToolScheduler', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('only creates one instance of CoreToolScheduler even if props change', () => { + const onComplete = vi.fn(); + const getPreferredEditor = vi.fn(); + const onEditorClose = vi.fn(); + const config = {} as Config; + + const { rerender } = renderHook( + (props) => + useReactToolScheduler( + props.onComplete, + props.config, + props.getPreferredEditor, + props.onEditorClose, + ), + { + initialProps: { + onComplete, + config, + getPreferredEditor, + onEditorClose, + }, + }, + ); + + expect(mockCoreToolScheduler).toHaveBeenCalledTimes(1); + + // Rerender with a new onComplete function + const newOnComplete = vi.fn(); + rerender({ + onComplete: newOnComplete, + config, + getPreferredEditor, + onEditorClose, + }); + expect(mockCoreToolScheduler).toHaveBeenCalledTimes(1); + + // Rerender with a new getPreferredEditor function + const newGetPreferredEditor = vi.fn(); + rerender({ + onComplete: newOnComplete, + config, + getPreferredEditor: newGetPreferredEditor, + onEditorClose, + }); + expect(mockCoreToolScheduler).toHaveBeenCalledTimes(1); + + // Rerender with a new onEditorClose function + const newOnEditorClose = vi.fn(); + rerender({ + onComplete: newOnComplete, + config, + getPreferredEditor: newGetPreferredEditor, + onEditorClose: newOnEditorClose, + }); + expect(mockCoreToolScheduler).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/cli/src/ui/hooks/useReactToolScheduler.ts b/packages/cli/src/ui/hooks/useReactToolScheduler.ts index d78b66e142..883690d79a 100644 --- a/packages/cli/src/ui/hooks/useReactToolScheduler.ts +++ b/packages/cli/src/ui/hooks/useReactToolScheduler.ts @@ -21,7 +21,7 @@ import type { EditorType, } from '@google/gemini-cli-core'; import { CoreToolScheduler, debugLogger } from '@google/gemini-cli-core'; -import { useCallback, useState, useMemo } from 'react'; +import { useCallback, useState, useMemo, useEffect, useRef } from 'react'; import type { HistoryItemToolGroup, IndividualToolCallDisplay, @@ -72,6 +72,23 @@ export function useReactToolScheduler( TrackedToolCall[] >([]); + // Store callbacks in refs to keep them up-to-date without causing re-renders. + const onCompleteRef = useRef(onComplete); + const getPreferredEditorRef = useRef(getPreferredEditor); + const onEditorCloseRef = useRef(onEditorClose); + + useEffect(() => { + onCompleteRef.current = onComplete; + }, [onComplete]); + + useEffect(() => { + getPreferredEditorRef.current = getPreferredEditor; + }, [getPreferredEditor]); + + useEffect(() => { + onEditorCloseRef.current = onEditorClose; + }, [onEditorClose]); + const outputUpdateHandler: OutputUpdateHandler = useCallback( (toolCallId, outputChunk) => { setToolCallsForDisplay((prevCalls) => @@ -89,9 +106,9 @@ export function useReactToolScheduler( const allToolCallsCompleteHandler: AllToolCallsCompleteHandler = useCallback( async (completedToolCalls) => { - await onComplete(completedToolCalls); + await onCompleteRef.current(completedToolCalls); }, - [onComplete], + [], ); const toolCallsUpdateHandler: ToolCallsUpdateHandler = useCallback( @@ -130,24 +147,29 @@ export function useReactToolScheduler( [setToolCallsForDisplay], ); + const stableGetPreferredEditor = useCallback( + () => getPreferredEditorRef.current(), + [], + ); + const stableOnEditorClose = useCallback(() => onEditorCloseRef.current(), []); + const scheduler = useMemo( () => new CoreToolScheduler({ outputUpdateHandler, onAllToolCallsComplete: allToolCallsCompleteHandler, onToolCallsUpdate: toolCallsUpdateHandler, - getPreferredEditor, + getPreferredEditor: stableGetPreferredEditor, config, - onEditorClose, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any), + onEditorClose: stableOnEditorClose, + }), [ config, outputUpdateHandler, allToolCallsCompleteHandler, toolCallsUpdateHandler, - getPreferredEditor, - onEditorClose, + stableGetPreferredEditor, + stableOnEditorClose, ], ); diff --git a/packages/core/src/core/coreToolScheduler.ts b/packages/core/src/core/coreToolScheduler.ts index 31265690dc..6c76f4aa5c 100644 --- a/packages/core/src/core/coreToolScheduler.ts +++ b/packages/core/src/core/coreToolScheduler.ts @@ -10,7 +10,6 @@ import type { ToolCallConfirmationDetails, ToolResult, ToolResultDisplay, - ToolRegistry, EditorType, Config, ToolConfirmationPayload, @@ -332,7 +331,6 @@ interface CoreToolSchedulerOptions { } export class CoreToolScheduler { - private toolRegistry: ToolRegistry; private toolCalls: ToolCall[] = []; private outputUpdateHandler?: OutputUpdateHandler; private onAllToolCallsComplete?: AllToolCallsCompleteHandler; @@ -351,7 +349,6 @@ export class CoreToolScheduler { constructor(options: CoreToolSchedulerOptions) { this.config = options.config; - this.toolRegistry = options.config.getToolRegistry(); this.outputUpdateHandler = options.outputUpdateHandler; this.onAllToolCallsComplete = options.onAllToolCallsComplete; this.onToolCallsUpdate = options.onToolCallsUpdate; @@ -603,7 +600,7 @@ export class CoreToolScheduler { * @returns A suggestion string like " Did you mean 'tool'?" or " Did you mean one of: 'tool1', 'tool2'?", or an empty string if no suggestions are found. */ private getToolSuggestion(unknownToolName: string, topN = 3): string { - const allToolNames = this.toolRegistry.getAllToolNames(); + const allToolNames = this.config.getToolRegistry().getAllToolNames(); const matches = allToolNames.map((toolName) => ({ name: toolName, @@ -680,7 +677,9 @@ export class CoreToolScheduler { const newToolCalls: ToolCall[] = requestsToProcess.map( (reqInfo): ToolCall => { - const toolInstance = this.toolRegistry.getTool(reqInfo.name); + const toolInstance = this.config + .getToolRegistry() + .getTool(reqInfo.name); if (!toolInstance) { const suggestion = this.getToolSuggestion(reqInfo.name); const errorMessage = `Tool "${reqInfo.name}" not found in registry. Tools must use the exact names that are registered.${suggestion}`; From 7787a31f81ce17fe1a0e96c0c625b8f63e2068b7 Mon Sep 17 00:00:00 2001 From: shishu314 Date: Thu, 23 Oct 2025 13:14:40 -0400 Subject: [PATCH 02/12] feat(infra) - Make merge group and pushes run chained e2e (#11796) Co-authored-by: gemini-cli-robot --- .github/workflows/test_chained_e2e.yml | 58 +++++++++++++++++++------- 1 file changed, 42 insertions(+), 16 deletions(-) diff --git a/.github/workflows/test_chained_e2e.yml b/.github/workflows/test_chained_e2e.yml index 6e0328475d..ba156e316f 100644 --- a/.github/workflows/test_chained_e2e.yml +++ b/.github/workflows/test_chained_e2e.yml @@ -1,6 +1,10 @@ name: 'Test Chained E2E' on: + push: + branches: + - 'main' + merge_group: workflow_run: workflows: ['Trigger E2E'] types: ['completed'] @@ -36,11 +40,12 @@ jobs: download_repo_name: runs-on: 'gemini-cli-ubuntu-16-core' + if: "github.event_name == 'workflow_dispatch' || github.event_name == 'workflow_run'" outputs: repo_name: '${{ steps.output-repo-name.outputs.repo_name }}' steps: - name: 'Mock Repo Artifact' - if: "${{ github.event_name != 'workflow_run' }}" + if: "${{ github.event_name == 'workflow_dispatch' }}" env: REPO_NAME: '${{ github.event.inputs.repo_name }}' run: | @@ -71,24 +76,44 @@ jobs: const repo_name = String(fs.readFileSync(path.join(temp, 'repo_name'))); core.setOutput('repo_name', repo_name); + parse_run_context: + name: 'Parse run context' + runs-on: 'gemini-cli-ubuntu-16-core' + needs: 'download_repo_name' + if: 'always()' + outputs: + repository: '${{ steps.set_context.outputs.REPO }}' + sha: '${{ steps.set_context.outputs.SHA }}' + steps: + - id: 'set_context' + name: 'Set dynamic repository and SHA' + env: + REPO: '${{ needs.download_repo_name.outputs.repo_name || github.repository }}' + SHA: '${{ github.event.inputs.head_sha || github.event.workflow_run.head_sha || github.sha }}' + shell: 'bash' + run: | + echo "REPO=$REPO" >> "$GITHUB_OUTPUT" + echo "SHA=$SHA" >> "$GITHUB_OUTPUT" + set_pending_status: runs-on: 'gemini-cli-ubuntu-16-core' + if: "github.event_name == 'workflow_dispatch' || github.event_name == 'workflow_run'" needs: - - 'download_repo_name' + - 'parse_run_context' steps: - name: 'Set pending status' uses: 'myrotvorets/set-commit-status-action@16037e056d73b2d3c88e37e393ff369047f70886' # ratchet:myrotvorets/set-commit-status-action@master if: 'always()' with: allowForks: 'true' - repo: '${{ needs.download_repo_name.outputs.repo_name }}' - sha: '${{ github.event.inputs.head_sha || github.event.workflow_run.head_sha }}' + repo: '${{ needs.parse_run_context.outputs.repository }}' + sha: '${{ needs.parse_run_context.outputs.sha }}' token: '${{ secrets.GITHUB_TOKEN }}' status: 'pending' e2e_linux: name: 'E2E Test (Linux) - ${{ matrix.sandbox }}' - needs: 'download_repo_name' + needs: 'parse_run_context' runs-on: 'gemini-cli-ubuntu-16-core' strategy: fail-fast: false @@ -103,8 +128,8 @@ jobs: - name: 'Checkout' uses: 'actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955' # ratchet:actions/checkout@v5 with: - ref: '${{ github.event.inputs.head_sha || github.event.workflow_run.head_sha }}' - repository: '${{ needs.download_repo_name.outputs.repo_name }}' + ref: '${{ needs.parse_run_context.outputs.sha }}' + repository: '${{ needs.parse_run_context.outputs.repository }}' - name: 'Set up Node.js ${{ matrix.node-version }}' uses: 'actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020' # ratchet:actions-node@v4 @@ -136,14 +161,14 @@ jobs: e2e_mac: name: 'E2E Test (macOS)' - needs: 'download_repo_name' + needs: 'parse_run_context' runs-on: 'macos-latest' steps: - name: 'Checkout' uses: 'actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955' # ratchet:actions/checkout@v5 with: - ref: '${{ github.event.inputs.head_sha || github.event.workflow_run.head_sha }}' - repository: '${{ needs.download_repo_name.outputs.repo_name }}' + ref: '${{ needs.parse_run_context.outputs.sha }}' + repository: '${{ needs.parse_run_context.outputs.repository }}' - name: 'Set up Node.js 20.x' uses: 'actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020' # ratchet:actions-node@v4 @@ -173,7 +198,7 @@ jobs: name: 'Slow E2E - Win' needs: - 'merge_queue_skipper' - - 'download_repo_name' + - 'parse_run_context' if: | needs.merge_queue_skipper.outputs.skip == 'false' runs-on: 'gemini-cli-windows-16-core' @@ -183,8 +208,8 @@ jobs: - name: 'Checkout' uses: 'actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955' # ratchet:actions/checkout@v5 with: - ref: '${{ github.event.inputs.head_sha || github.event.workflow_run.head_sha }}' - repository: '${{ needs.download_repo_name.outputs.repo_name }}' + ref: '${{ needs.parse_run_context.outputs.sha }}' + repository: '${{ needs.parse_run_context.outputs.repository }}' - name: 'Set up Node.js 20.x' uses: 'actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020' # ratchet:actions-node@v4 @@ -251,8 +276,9 @@ jobs: set_workflow_status: runs-on: 'gemini-cli-ubuntu-16-core' + if: "github.event_name == 'workflow_dispatch' || github.event_name == 'workflow_run'" needs: - - 'download_repo_name' + - 'parse_run_context' - 'e2e' steps: - name: 'Set workflow status' @@ -260,7 +286,7 @@ jobs: if: 'always()' with: allowForks: 'true' - repo: '${{ needs.download_repo_name.outputs.repo_name }}' - sha: '${{ github.event.inputs.head_sha || github.event.workflow_run.head_sha }}' + repo: '${{ needs.parse_run_context.outputs.repository }}' + sha: '${{ needs.parse_run_context.outputs.sha }}' token: '${{ secrets.GITHUB_TOKEN }}' status: '${{ job.status }}' From 3a501196f0f49f693a531a56e43d56f41bd872b9 Mon Sep 17 00:00:00 2001 From: Abhi <43648792+abhipatel12@users.noreply.github.com> Date: Thu, 23 Oct 2025 14:14:14 -0400 Subject: [PATCH 03/12] feat(ux): Surface internal errors via unified event system (#11803) --- packages/cli/src/config/settings.test.ts | 78 ++++++++++ packages/cli/src/config/settings.ts | 7 +- packages/cli/src/nonInteractiveCli.test.ts | 157 +++++++++++++++++++- packages/cli/src/nonInteractiveCli.ts | 19 +++ packages/cli/src/ui/AppContainer.test.tsx | 93 +++++++++++- packages/cli/src/ui/AppContainer.tsx | 50 +++++++ packages/core/src/index.ts | 1 + packages/core/src/utils/events.test.ts | 159 +++++++++++++++++++++ packages/core/src/utils/events.ts | 115 +++++++++++++++ 9 files changed, 676 insertions(+), 3 deletions(-) create mode 100644 packages/core/src/utils/events.test.ts create mode 100644 packages/core/src/utils/events.ts diff --git a/packages/cli/src/config/settings.test.ts b/packages/cli/src/config/settings.test.ts index c3b7cf748e..2f7b358f6e 100644 --- a/packages/cli/src/config/settings.test.ts +++ b/packages/cli/src/config/settings.test.ts @@ -64,9 +64,12 @@ import { loadEnvironment, migrateDeprecatedSettings, SettingScope, + saveSettings, + type SettingsFile, } from './settings.js'; import { FatalConfigError, GEMINI_DIR, Storage } from '@google/gemini-cli-core'; import { ExtensionEnablementManager } from './extensions/extensionEnablement.js'; +import { updateSettingsFilePreservingFormat } from '../utils/commentJson.js'; const MOCK_WORKSPACE_DIR = '/mock/workspace'; // Use the (mocked) GEMINI_DIR for consistency @@ -96,6 +99,23 @@ vi.mock('fs', async (importOriginal) => { vi.mock('./extension.js'); +const mockCoreEvents = vi.hoisted(() => ({ + emitFeedback: vi.fn(), +})); + +vi.mock('@google/gemini-cli-core', async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + coreEvents: mockCoreEvents, + }; +}); + +vi.mock('../utils/commentJson.js', () => ({ + updateSettingsFilePreservingFormat: vi.fn(), +})); + vi.mock('strip-json-comments', () => ({ default: vi.fn((content) => content), })); @@ -2495,4 +2515,62 @@ describe('Settings Loading and Merging', () => { expect(setValueSpy).not.toHaveBeenCalled(); }); }); + + describe('saveSettings', () => { + it('should save settings using updateSettingsFilePreservingFormat', () => { + const mockUpdateSettings = vi.mocked(updateSettingsFilePreservingFormat); + const settingsFile = { + path: '/mock/settings.json', + settings: { ui: { theme: 'dark' } }, + originalSettings: { ui: { theme: 'dark' } }, + } as unknown as SettingsFile; + + saveSettings(settingsFile); + + expect(mockUpdateSettings).toHaveBeenCalledWith('/mock/settings.json', { + ui: { theme: 'dark' }, + }); + }); + + it('should create directory if it does not exist', () => { + const mockFsExistsSync = vi.mocked(fs.existsSync); + const mockFsMkdirSync = vi.mocked(fs.mkdirSync); + mockFsExistsSync.mockReturnValue(false); + + const settingsFile = { + path: '/mock/new/dir/settings.json', + settings: {}, + originalSettings: {}, + } as unknown as SettingsFile; + + saveSettings(settingsFile); + + expect(mockFsExistsSync).toHaveBeenCalledWith('/mock/new/dir'); + expect(mockFsMkdirSync).toHaveBeenCalledWith('/mock/new/dir', { + recursive: true, + }); + }); + + it('should emit error feedback if saving fails', () => { + const mockUpdateSettings = vi.mocked(updateSettingsFilePreservingFormat); + const error = new Error('Write failed'); + mockUpdateSettings.mockImplementation(() => { + throw error; + }); + + const settingsFile = { + path: '/mock/settings.json', + settings: {}, + originalSettings: {}, + } as unknown as SettingsFile; + + saveSettings(settingsFile); + + expect(mockCoreEvents.emitFeedback).toHaveBeenCalledWith( + 'error', + 'There was an error saving your latest settings changes.', + error, + ); + }); + }); }); diff --git a/packages/cli/src/config/settings.ts b/packages/cli/src/config/settings.ts index 0835cdd178..2818c9a214 100644 --- a/packages/cli/src/config/settings.ts +++ b/packages/cli/src/config/settings.ts @@ -15,6 +15,7 @@ import { GEMINI_DIR, getErrorMessage, Storage, + coreEvents, } from '@google/gemini-cli-core'; import stripJsonComments from 'strip-json-comments'; import { DefaultLight } from '../ui/themes/default-light.js'; @@ -799,6 +800,10 @@ export function saveSettings(settingsFile: SettingsFile): void { settingsToSave as Record, ); } catch (error) { - console.error('Error saving user settings file:', error); + coreEvents.emitFeedback( + 'error', + 'There was an error saving your latest settings changes.', + error, + ); } } diff --git a/packages/cli/src/nonInteractiveCli.test.ts b/packages/cli/src/nonInteractiveCli.test.ts index d82ab2108c..da5d097c64 100644 --- a/packages/cli/src/nonInteractiveCli.test.ts +++ b/packages/cli/src/nonInteractiveCli.test.ts @@ -11,6 +11,7 @@ import type { SessionMetrics, AnyDeclarativeTool, AnyToolInvocation, + UserFeedbackPayload, } from '@google/gemini-cli-core'; import { executeToolCall, @@ -20,14 +21,32 @@ import { OutputFormat, uiTelemetryService, FatalInputError, + CoreEvent, } from '@google/gemini-cli-core'; import type { Part } from '@google/genai'; import { runNonInteractive } from './nonInteractiveCli.js'; -import { vi, type Mock, type MockInstance } from 'vitest'; +import { + describe, + it, + expect, + beforeEach, + afterEach, + vi, + type Mock, + type MockInstance, +} from 'vitest'; import type { LoadedSettings } from './config/settings.js'; // Mock core modules vi.mock('./ui/hooks/atCommandProcessor.js'); + +const mockCoreEvents = vi.hoisted(() => ({ + on: vi.fn(), + off: vi.fn(), + drainFeedbackBacklog: vi.fn(), + emit: vi.fn(), +})); + vi.mock('@google/gemini-cli-core', async (importOriginal) => { const original = await importOriginal(); @@ -48,6 +67,7 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => { uiTelemetryService: { getMetrics: vi.fn(), }, + coreEvents: mockCoreEvents, }; }); @@ -70,6 +90,7 @@ describe('runNonInteractive', () => { let mockShutdownTelemetry: Mock; let consoleErrorSpy: MockInstance; let processStdoutSpy: MockInstance; + let processStderrSpy: MockInstance; let mockGeminiClient: { sendMessageStream: Mock; getChatRecordingService: Mock; @@ -87,6 +108,9 @@ describe('runNonInteractive', () => { processStdoutSpy = vi .spyOn(process.stdout, 'write') .mockImplementation(() => true); + processStderrSpy = vi + .spyOn(process.stderr, 'write') + .mockImplementation(() => true); vi.spyOn(process, 'exit').mockImplementation((code) => { throw new Error(`process.exit(${code}) called`); }); @@ -1051,4 +1075,135 @@ describe('runNonInteractive', () => { ); expect(processStdoutSpy).toHaveBeenCalledWith('file.txt'); }); + + describe('CoreEvents Integration', () => { + it('subscribes to UserFeedback and drains backlog on start', async () => { + const events: ServerGeminiStreamEvent[] = [ + { + type: GeminiEventType.Finished, + value: { reason: undefined, usageMetadata: { totalTokenCount: 0 } }, + }, + ]; + mockGeminiClient.sendMessageStream.mockReturnValue( + createStreamFromEvents(events), + ); + + await runNonInteractive( + mockConfig, + mockSettings, + 'test', + 'prompt-id-events', + ); + + expect(mockCoreEvents.on).toHaveBeenCalledWith( + CoreEvent.UserFeedback, + expect.any(Function), + ); + expect(mockCoreEvents.drainFeedbackBacklog).toHaveBeenCalledTimes(1); + }); + + it('unsubscribes from UserFeedback on finish', async () => { + const events: ServerGeminiStreamEvent[] = [ + { + type: GeminiEventType.Finished, + value: { reason: undefined, usageMetadata: { totalTokenCount: 0 } }, + }, + ]; + mockGeminiClient.sendMessageStream.mockReturnValue( + createStreamFromEvents(events), + ); + + await runNonInteractive( + mockConfig, + mockSettings, + 'test', + 'prompt-id-events', + ); + + expect(mockCoreEvents.off).toHaveBeenCalledWith( + CoreEvent.UserFeedback, + expect.any(Function), + ); + }); + + it('logs to process.stderr when UserFeedback event is received', async () => { + const events: ServerGeminiStreamEvent[] = [ + { + type: GeminiEventType.Finished, + value: { reason: undefined, usageMetadata: { totalTokenCount: 0 } }, + }, + ]; + mockGeminiClient.sendMessageStream.mockReturnValue( + createStreamFromEvents(events), + ); + + await runNonInteractive( + mockConfig, + mockSettings, + 'test', + 'prompt-id-events', + ); + + // Get the registered handler + const handler = mockCoreEvents.on.mock.calls.find( + (call: unknown[]) => call[0] === CoreEvent.UserFeedback, + )?.[1]; + expect(handler).toBeDefined(); + + // Simulate an event + const payload: UserFeedbackPayload = { + severity: 'error', + message: 'Test error message', + }; + handler(payload); + + expect(processStderrSpy).toHaveBeenCalledWith( + '[ERROR] Test error message\n', + ); + }); + + it('logs optional error object to process.stderr in debug mode', async () => { + vi.mocked(mockConfig.getDebugMode).mockReturnValue(true); + const events: ServerGeminiStreamEvent[] = [ + { + type: GeminiEventType.Finished, + value: { reason: undefined, usageMetadata: { totalTokenCount: 0 } }, + }, + ]; + mockGeminiClient.sendMessageStream.mockReturnValue( + createStreamFromEvents(events), + ); + + await runNonInteractive( + mockConfig, + mockSettings, + 'test', + 'prompt-id-events', + ); + + // Get the registered handler + const handler = mockCoreEvents.on.mock.calls.find( + (call: unknown[]) => call[0] === CoreEvent.UserFeedback, + )?.[1]; + expect(handler).toBeDefined(); + + // Simulate an event with error object + const errorObj = new Error('Original error'); + // Mock stack for deterministic testing + errorObj.stack = 'Error: Original error\n at test'; + const payload: UserFeedbackPayload = { + severity: 'warning', + message: 'Test warning message', + error: errorObj, + }; + handler(payload); + + expect(processStderrSpy).toHaveBeenCalledWith( + '[WARNING] Test warning message\n', + ); + expect(processStderrSpy).toHaveBeenCalledWith( + 'Error: Original error\n at test\n', + ); + }); + }); }); diff --git a/packages/cli/src/nonInteractiveCli.ts b/packages/cli/src/nonInteractiveCli.ts index f92b17e800..2d8d845114 100644 --- a/packages/cli/src/nonInteractiveCli.ts +++ b/packages/cli/src/nonInteractiveCli.ts @@ -8,6 +8,7 @@ import type { Config, ToolCallRequestInfo, CompletedToolCall, + UserFeedbackPayload, } from '@google/gemini-cli-core'; import { isSlashCommand } from './ui/utils/commandUtils.js'; import type { LoadedSettings } from './config/settings.js'; @@ -24,6 +25,8 @@ import { JsonStreamEventType, uiTelemetryService, debugLogger, + coreEvents, + CoreEvent, } from '@google/gemini-cli-core'; import type { Content, Part } from '@google/genai'; @@ -50,6 +53,18 @@ export async function runNonInteractive( debugMode: config.getDebugMode(), }); + const handleUserFeedback = (payload: UserFeedbackPayload) => { + const prefix = payload.severity.toUpperCase(); + process.stderr.write(`[${prefix}] ${payload.message}\n`); + if (payload.error && config.getDebugMode()) { + const errorToLog = + payload.error instanceof Error + ? payload.error.stack || payload.error.message + : String(payload.error); + process.stderr.write(`${errorToLog}\n`); + } + }; + const startTime = Date.now(); const streamFormatter = config.getOutputFormat() === OutputFormat.STREAM_JSON @@ -58,6 +73,9 @@ export async function runNonInteractive( try { consolePatcher.patch(); + coreEvents.on(CoreEvent.UserFeedback, handleUserFeedback); + coreEvents.drainFeedbackBacklog(); + // Handle EPIPE errors when the output is piped to a command that closes early. process.stdout.on('error', (err: NodeJS.ErrnoException) => { if (err.code === 'EPIPE') { @@ -287,6 +305,7 @@ export async function runNonInteractive( handleError(error, config); } finally { consolePatcher.cleanup(); + coreEvents.off(CoreEvent.UserFeedback, handleUserFeedback); if (isTelemetrySdkInitialized()) { await shutdownTelemetry(config); } diff --git a/packages/cli/src/ui/AppContainer.test.tsx b/packages/cli/src/ui/AppContainer.test.tsx index d12dc918fc..5864437880 100644 --- a/packages/cli/src/ui/AppContainer.test.tsx +++ b/packages/cli/src/ui/AppContainer.test.tsx @@ -15,7 +15,29 @@ import { } from 'vitest'; import { render, cleanup } from 'ink-testing-library'; import { AppContainer } from './AppContainer.js'; -import { type Config, makeFakeConfig } from '@google/gemini-cli-core'; +import { + type Config, + makeFakeConfig, + CoreEvent, + type UserFeedbackPayload, +} from '@google/gemini-cli-core'; + +// Mock coreEvents +const mockCoreEvents = vi.hoisted(() => ({ + on: vi.fn(), + off: vi.fn(), + drainFeedbackBacklog: vi.fn(), + emit: vi.fn(), +})); + +vi.mock('@google/gemini-cli-core', async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + coreEvents: mockCoreEvents, + }; +}); import type { LoadedSettings } from '../config/settings.js'; import type { InitializationResult } from '../core/initializer.js'; import { useQuotaAndFallback } from './hooks/useQuotaAndFallback.js'; @@ -1293,4 +1315,73 @@ describe('AppContainer State Management', () => { expect(mockCloseModelDialog).toHaveBeenCalled(); }); }); + + describe('CoreEvents Integration', () => { + it('subscribes to UserFeedback and drains backlog on mount', () => { + render( + , + ); + + expect(mockCoreEvents.on).toHaveBeenCalledWith( + CoreEvent.UserFeedback, + expect.any(Function), + ); + expect(mockCoreEvents.drainFeedbackBacklog).toHaveBeenCalledTimes(1); + }); + + it('unsubscribes from UserFeedback on unmount', () => { + const { unmount } = render( + , + ); + + unmount(); + + expect(mockCoreEvents.off).toHaveBeenCalledWith( + CoreEvent.UserFeedback, + expect.any(Function), + ); + }); + + it('adds history item when UserFeedback event is received', () => { + render( + , + ); + + // Get the registered handler + const handler = mockCoreEvents.on.mock.calls.find( + (call: unknown[]) => call[0] === CoreEvent.UserFeedback, + )?.[1]; + expect(handler).toBeDefined(); + + // Simulate an event + const payload: UserFeedbackPayload = { + severity: 'error', + message: 'Test error message', + }; + handler(payload); + + expect(mockedUseHistory().addItem).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'error', + text: 'Test error message', + }), + expect.any(Number), + ); + }); + }); }); diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index 28ebc47d71..c8de2a27ec 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -34,6 +34,7 @@ import { type IdeInfo, type IdeContext, type UserTierId, + type UserFeedbackPayload, DEFAULT_GEMINI_FLASH_MODEL, IdeClient, ideContextStore, @@ -44,6 +45,8 @@ import { recordExitFail, ShellExecutionService, debugLogger, + coreEvents, + CoreEvent, } from '@google/gemini-cli-core'; import { validateAuthMethod } from '../config/auth.js'; import { loadHierarchicalGeminiMemory } from '../config/config.js'; @@ -1066,6 +1069,53 @@ Logging in with Google... Please restart Gemini CLI to continue. stdout, ]); + useEffect(() => { + const handleUserFeedback = (payload: UserFeedbackPayload) => { + let type: MessageType; + switch (payload.severity) { + case 'error': + type = MessageType.ERROR; + break; + case 'warning': + type = MessageType.WARNING; + break; + case 'info': + type = MessageType.INFO; + break; + default: + throw new Error( + `Unexpected severity for user feedback: ${payload.severity}`, + ); + } + + historyManager.addItem( + { + type, + text: payload.message, + }, + Date.now(), + ); + + // If there is an attached error object, log it to the debug drawer. + if (payload.error) { + debugLogger.warn( + `[Feedback Details for "${payload.message}"]`, + payload.error, + ); + } + }; + + coreEvents.on(CoreEvent.UserFeedback, handleUserFeedback); + + // Flush any messages that happened during startup before this component + // mounted. + coreEvents.drainFeedbackBacklog(); + + return () => { + coreEvents.off(CoreEvent.UserFeedback, handleUserFeedback); + }; + }, [historyManager]); + const filteredConsoleMessages = useMemo(() => { if (config.getDebugMode()) { return consoleMessages; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index e2248b0c73..42ced4457f 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -64,6 +64,7 @@ export * from './utils/partUtils.js'; export * from './utils/promptIdContext.js'; export * from './utils/thoughtUtils.js'; export * from './utils/debugLogger.js'; +export * from './utils/events.js'; // Export services export * from './services/fileDiscoveryService.js'; diff --git a/packages/core/src/utils/events.test.ts b/packages/core/src/utils/events.test.ts new file mode 100644 index 0000000000..4a11263014 --- /dev/null +++ b/packages/core/src/utils/events.test.ts @@ -0,0 +1,159 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { + CoreEventEmitter, + CoreEvent, + type UserFeedbackPayload, +} from './events.js'; + +describe('CoreEventEmitter', () => { + let events: CoreEventEmitter; + + beforeEach(() => { + events = new CoreEventEmitter(); + }); + + it('should emit feedback immediately when a listener is present', () => { + const listener = vi.fn(); + events.on(CoreEvent.UserFeedback, listener); + + const payload = { + severity: 'info' as const, + message: 'Test message', + }; + + events.emitFeedback(payload.severity, payload.message); + + expect(listener).toHaveBeenCalledTimes(1); + expect(listener).toHaveBeenCalledWith(expect.objectContaining(payload)); + }); + + it('should buffer feedback when no listener is present', () => { + const listener = vi.fn(); + const payload = { + severity: 'warning' as const, + message: 'Buffered message', + }; + + // Emit while no listeners attached + events.emitFeedback(payload.severity, payload.message); + expect(listener).not.toHaveBeenCalled(); + + // Attach listener and drain + events.on(CoreEvent.UserFeedback, listener); + events.drainFeedbackBacklog(); + + expect(listener).toHaveBeenCalledTimes(1); + expect(listener).toHaveBeenCalledWith(expect.objectContaining(payload)); + }); + + it('should respect the backlog size limit and maintain FIFO order', () => { + const listener = vi.fn(); + const MAX_BACKLOG_SIZE = 10000; + + for (let i = 0; i < MAX_BACKLOG_SIZE + 10; i++) { + events.emitFeedback('info', `Message ${i}`); + } + + events.on(CoreEvent.UserFeedback, listener); + events.drainFeedbackBacklog(); + + expect(listener).toHaveBeenCalledTimes(MAX_BACKLOG_SIZE); + // Verify strictly that the FIRST call was Message 10 (0-9 dropped) + expect(listener.mock.calls[0][0]).toMatchObject({ message: 'Message 10' }); + // Verify strictly that the LAST call was Message 109 + expect(listener.mock.lastCall?.[0]).toMatchObject({ + message: `Message ${MAX_BACKLOG_SIZE + 9}`, + }); + }); + + it('should clear the backlog after draining', () => { + const listener = vi.fn(); + events.emitFeedback('error', 'Test error'); + + events.on(CoreEvent.UserFeedback, listener); + events.drainFeedbackBacklog(); + expect(listener).toHaveBeenCalledTimes(1); + + listener.mockClear(); + events.drainFeedbackBacklog(); + expect(listener).not.toHaveBeenCalled(); + }); + + it('should include optional error object in payload', () => { + const listener = vi.fn(); + events.on(CoreEvent.UserFeedback, listener); + + const error = new Error('Original error'); + events.emitFeedback('error', 'Something went wrong', error); + + expect(listener).toHaveBeenCalledWith( + expect.objectContaining({ + severity: 'error', + message: 'Something went wrong', + error, + }), + ); + }); + + it('should handle multiple listeners correctly', () => { + const listenerA = vi.fn(); + const listenerB = vi.fn(); + + events.on(CoreEvent.UserFeedback, listenerA); + events.on(CoreEvent.UserFeedback, listenerB); + + events.emitFeedback('info', 'Broadcast message'); + + expect(listenerA).toHaveBeenCalledTimes(1); + expect(listenerB).toHaveBeenCalledTimes(1); + }); + + it('should stop receiving events after off() is called', () => { + const listener = vi.fn(); + events.on(CoreEvent.UserFeedback, listener); + + events.emitFeedback('info', 'First message'); + expect(listener).toHaveBeenCalledTimes(1); + + events.off(CoreEvent.UserFeedback, listener); + events.emitFeedback('info', 'Second message'); + expect(listener).toHaveBeenCalledTimes(1); // Still 1 + }); + + it('should handle re-entrant feedback emission during draining safely', () => { + events.emitFeedback('info', 'Buffered 1'); + events.emitFeedback('info', 'Buffered 2'); + + const listener = vi.fn((payload: UserFeedbackPayload) => { + // When 'Buffered 1' is received, immediately emit another event. + if (payload.message === 'Buffered 1') { + events.emitFeedback('warning', 'Re-entrant message'); + } + }); + + events.on(CoreEvent.UserFeedback, listener); + events.drainFeedbackBacklog(); + + // Expectation with atomic snapshot: + // 1. loop starts with ['Buffered 1', 'Buffered 2'] + // 2. emits 'Buffered 1' + // 3. listener fires for 'Buffered 1', calls emitFeedback('Re-entrant') + // 4. emitFeedback sees listener attached, emits 'Re-entrant' synchronously + // 5. listener fires for 'Re-entrant' + // 6. loop continues, emits 'Buffered 2' + // 7. listener fires for 'Buffered 2' + + expect(listener).toHaveBeenCalledTimes(3); + expect(listener.mock.calls[0][0]).toMatchObject({ message: 'Buffered 1' }); + expect(listener.mock.calls[1][0]).toMatchObject({ + message: 'Re-entrant message', + }); + expect(listener.mock.calls[2][0]).toMatchObject({ message: 'Buffered 2' }); + }); +}); diff --git a/packages/core/src/utils/events.ts b/packages/core/src/utils/events.ts new file mode 100644 index 0000000000..76038560d8 --- /dev/null +++ b/packages/core/src/utils/events.ts @@ -0,0 +1,115 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { EventEmitter } from 'node:events'; + +/** + * Defines the severity level for user-facing feedback. + * This maps loosely to UI `MessageType` + */ +export type FeedbackSeverity = 'info' | 'warning' | 'error'; + +/** + * Payload for the 'user-feedback' event. + */ +export interface UserFeedbackPayload { + /** + * The severity level determines how the message is rendered in the UI + * (e.g. colored text, specific icon). + */ + severity: FeedbackSeverity; + /** + * The main message to display to the user in the chat history or stdout. + */ + message: string; + /** + * The original error object, if applicable. + * Listeners can use this to extract stack traces for debug logging + * or verbose output, while keeping the 'message' field clean for end users. + */ + error?: unknown; +} + +export enum CoreEvent { + UserFeedback = 'user-feedback', +} + +export class CoreEventEmitter extends EventEmitter { + private _feedbackBacklog: UserFeedbackPayload[] = []; + private static readonly MAX_BACKLOG_SIZE = 10000; + + constructor() { + super(); + } + + /** + * Sends actionable feedback to the user. + * Buffers automatically if the UI hasn't subscribed yet. + */ + emitFeedback( + severity: FeedbackSeverity, + message: string, + error?: unknown, + ): void { + const payload: UserFeedbackPayload = { severity, message, error }; + + if (this.listenerCount(CoreEvent.UserFeedback) === 0) { + if (this._feedbackBacklog.length >= CoreEventEmitter.MAX_BACKLOG_SIZE) { + this._feedbackBacklog.shift(); + } + this._feedbackBacklog.push(payload); + } else { + this.emit(CoreEvent.UserFeedback, payload); + } + } + + /** + * Flushes buffered messages. Call this immediately after primary UI listener + * subscribes. + */ + drainFeedbackBacklog(): void { + const backlog = [...this._feedbackBacklog]; + this._feedbackBacklog.length = 0; // Clear in-place + for (const payload of backlog) { + this.emit(CoreEvent.UserFeedback, payload); + } + } + + override on( + event: CoreEvent.UserFeedback, + listener: (payload: UserFeedbackPayload) => void, + ): this; + override on( + event: string | symbol, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + listener: (...args: any[]) => void, + ): this { + return super.on(event, listener); + } + + override off( + event: CoreEvent.UserFeedback, + listener: (payload: UserFeedbackPayload) => void, + ): this; + override off( + event: string | symbol, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + listener: (...args: any[]) => void, + ): this { + return super.off(event, listener); + } + + override emit( + event: CoreEvent.UserFeedback, + payload: UserFeedbackPayload, + ): boolean; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + override emit(event: string | symbol, ...args: any[]): boolean { + return super.emit(event, ...args); + } +} + +export const coreEvents = new CoreEventEmitter(); From c4c0c0d182f6fc05d2054f30c0e9067effee543e Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Thu, 23 Oct 2025 11:39:36 -0700 Subject: [PATCH 04/12] Create ExtensionManager class which manages all high level extension tasks (#11667) --- .../cli/src/commands/extensions/disable.ts | 28 +- .../cli/src/commands/extensions/enable.ts | 23 +- .../src/commands/extensions/install.test.ts | 16 +- .../cli/src/commands/extensions/install.ts | 26 +- packages/cli/src/commands/extensions/link.ts | 21 +- packages/cli/src/commands/extensions/list.ts | 22 +- .../cli/src/commands/extensions/uninstall.ts | 14 +- .../cli/src/commands/extensions/update.ts | 38 +- packages/cli/src/commands/mcp/list.test.ts | 22 +- packages/cli/src/commands/mcp/list.ts | 13 +- packages/cli/src/config/extension-manager.ts | 643 +++++++++++++ packages/cli/src/config/extension.test.ts | 563 +++++------- packages/cli/src/config/extension.ts | 846 +----------------- packages/cli/src/config/extensions/consent.ts | 162 ++++ .../config/extensions/extensionEnablement.ts | 2 +- .../extensions/extensionSettings.test.ts | 2 +- .../config/extensions/extensionSettings.ts | 2 +- .../cli/src/config/extensions/github.test.ts | 65 +- packages/cli/src/config/extensions/github.ts | 13 +- packages/cli/src/config/extensions/storage.ts | 47 + .../cli/src/config/extensions/update.test.ts | 113 +-- packages/cli/src/config/extensions/update.ts | 61 +- .../src/config/extensions/variableSchema.ts | 8 - .../cli/src/config/extensions/variables.ts | 7 + packages/cli/src/config/settings.test.ts | 48 +- packages/cli/src/config/settings.ts | 13 +- packages/cli/src/gemini.tsx | 31 +- .../cli/src/test-utils/createExtension.ts | 8 +- packages/cli/src/ui/AppContainer.tsx | 35 +- .../src/ui/hooks/useExtensionUpdates.test.ts | 67 +- .../cli/src/ui/hooks/useExtensionUpdates.ts | 59 +- 31 files changed, 1450 insertions(+), 1568 deletions(-) create mode 100644 packages/cli/src/config/extension-manager.ts create mode 100644 packages/cli/src/config/extensions/consent.ts create mode 100644 packages/cli/src/config/extensions/storage.ts diff --git a/packages/cli/src/commands/extensions/disable.ts b/packages/cli/src/commands/extensions/disable.ts index 53f8f145fd..184d11a410 100644 --- a/packages/cli/src/commands/extensions/disable.ts +++ b/packages/cli/src/commands/extensions/disable.ts @@ -5,11 +5,12 @@ */ import { type CommandModule } from 'yargs'; -import { disableExtension } from '../../config/extension.js'; -import { SettingScope } from '../../config/settings.js'; +import { loadSettings, SettingScope } from '../../config/settings.js'; import { getErrorMessage } from '../../utils/errors.js'; -import { ExtensionEnablementManager } from '../../config/extensions/extensionEnablement.js'; import { debugLogger } from '@google/gemini-cli-core'; +import { ExtensionManager } from '../../config/extension-manager.js'; +import { requestConsentNonInteractive } from '../../config/extensions/consent.js'; +import { promptForSetting } from '../../config/extensions/extensionSettings.js'; interface DisableArgs { name: string; @@ -17,20 +18,19 @@ interface DisableArgs { } export function handleDisable(args: DisableArgs) { - const extensionEnablementManager = new ExtensionEnablementManager(); + const workspaceDir = process.cwd(); + const extensionManager = new ExtensionManager({ + workspaceDir, + requestConsent: requestConsentNonInteractive, + requestSetting: promptForSetting, + loadedSettings: loadSettings(workspaceDir), + }); + try { if (args.scope?.toLowerCase() === 'workspace') { - disableExtension( - args.name, - SettingScope.Workspace, - extensionEnablementManager, - ); + extensionManager.disableExtension(args.name, SettingScope.Workspace); } else { - disableExtension( - args.name, - SettingScope.User, - extensionEnablementManager, - ); + extensionManager.disableExtension(args.name, SettingScope.User); } debugLogger.log( `Extension "${args.name}" successfully disabled for scope "${args.scope}".`, diff --git a/packages/cli/src/commands/extensions/enable.ts b/packages/cli/src/commands/extensions/enable.ts index f5245088d7..43523af372 100644 --- a/packages/cli/src/commands/extensions/enable.ts +++ b/packages/cli/src/commands/extensions/enable.ts @@ -5,14 +5,15 @@ */ import { type CommandModule } from 'yargs'; +import { loadSettings, SettingScope } from '../../config/settings.js'; +import { requestConsentNonInteractive } from '../../config/extensions/consent.js'; +import { ExtensionManager } from '../../config/extension-manager.js'; import { debugLogger, FatalConfigError, getErrorMessage, } from '@google/gemini-cli-core'; -import { enableExtension } from '../../config/extension.js'; -import { SettingScope } from '../../config/settings.js'; -import { ExtensionEnablementManager } from '../../config/extensions/extensionEnablement.js'; +import { promptForSetting } from '../../config/extensions/extensionSettings.js'; interface EnableArgs { name: string; @@ -20,16 +21,18 @@ interface EnableArgs { } export function handleEnable(args: EnableArgs) { - const extensionEnablementManager = new ExtensionEnablementManager(); + const workingDir = process.cwd(); + const extensionManager = new ExtensionManager({ + workspaceDir: workingDir, + requestConsent: requestConsentNonInteractive, + requestSetting: promptForSetting, + loadedSettings: loadSettings(workingDir), + }); try { if (args.scope?.toLowerCase() === 'workspace') { - enableExtension( - args.name, - SettingScope.Workspace, - extensionEnablementManager, - ); + extensionManager.enableExtension(args.name, SettingScope.Workspace); } else { - enableExtension(args.name, SettingScope.User, extensionEnablementManager); + extensionManager.enableExtension(args.name, SettingScope.User); } if (args.scope) { debugLogger.log( diff --git a/packages/cli/src/commands/extensions/install.test.ts b/packages/cli/src/commands/extensions/install.test.ts index 3f85015da1..7348bf89ec 100644 --- a/packages/cli/src/commands/extensions/install.test.ts +++ b/packages/cli/src/commands/extensions/install.test.ts @@ -12,11 +12,21 @@ const mockInstallOrUpdateExtension = vi.hoisted(() => vi.fn()); const mockRequestConsentNonInteractive = vi.hoisted(() => vi.fn()); const mockStat = vi.hoisted(() => vi.fn()); -vi.mock('../../config/extension.js', () => ({ - installOrUpdateExtension: mockInstallOrUpdateExtension, +vi.mock('../../config/extensions/consent.js', () => ({ requestConsentNonInteractive: mockRequestConsentNonInteractive, })); +vi.mock('../../config/extension-manager.ts', async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + ExtensionManager: vi.fn().mockImplementation(() => ({ + installOrUpdateExtension: mockInstallOrUpdateExtension, + })), + }; +}); + vi.mock('../../utils/errors.js', () => ({ getErrorMessage: vi.fn((error: Error) => error.message), })); @@ -54,7 +64,7 @@ describe('handleInstall', () => { mockInstallOrUpdateExtension.mockClear(); mockRequestConsentNonInteractive.mockClear(); mockStat.mockClear(); - vi.resetAllMocks(); + vi.clearAllMocks(); }); it('should install an extension from a http source', async () => { diff --git a/packages/cli/src/commands/extensions/install.ts b/packages/cli/src/commands/extensions/install.ts index 58120f084e..13c59a1855 100644 --- a/packages/cli/src/commands/extensions/install.ts +++ b/packages/cli/src/commands/extensions/install.ts @@ -5,17 +5,18 @@ */ import type { CommandModule } from 'yargs'; -import { - INSTALL_WARNING_MESSAGE, - installOrUpdateExtension, - requestConsentNonInteractive, -} from '../../config/extension.js'; import { debugLogger, type ExtensionInstallMetadata, } from '@google/gemini-cli-core'; import { getErrorMessage } from '../../utils/errors.js'; import { stat } from 'node:fs/promises'; +import { + INSTALL_WARNING_MESSAGE, + requestConsentNonInteractive, +} from '../../config/extensions/consent.js'; +import { ExtensionManager } from '../../config/extension-manager.js'; +import { loadSettings } from '../../config/settings.js'; import { promptForSetting } from '../../config/extensions/extensionSettings.js'; interface InstallArgs { @@ -67,13 +68,16 @@ export async function handleInstall(args: InstallArgs) { debugLogger.log('You have consented to the following:'); debugLogger.log(INSTALL_WARNING_MESSAGE); } - const name = await installOrUpdateExtension( - installMetadata, + + const workspaceDir = process.cwd(); + const extensionManager = new ExtensionManager({ + workspaceDir, requestConsent, - process.cwd(), - undefined, - promptForSetting, - ); + requestSetting: promptForSetting, + loadedSettings: loadSettings(workspaceDir), + }); + const name = + await extensionManager.installOrUpdateExtension(installMetadata); debugLogger.log(`Extension "${name}" installed successfully and enabled.`); } catch (error) { debugLogger.error(getErrorMessage(error)); diff --git a/packages/cli/src/commands/extensions/link.ts b/packages/cli/src/commands/extensions/link.ts index 6bbfcebd7e..9f0693cd7e 100644 --- a/packages/cli/src/commands/extensions/link.ts +++ b/packages/cli/src/commands/extensions/link.ts @@ -5,16 +5,16 @@ */ import type { CommandModule } from 'yargs'; -import { - installOrUpdateExtension, - requestConsentNonInteractive, -} from '../../config/extension.js'; import { debugLogger, type ExtensionInstallMetadata, } from '@google/gemini-cli-core'; import { getErrorMessage } from '../../utils/errors.js'; +import { requestConsentNonInteractive } from '../../config/extensions/consent.js'; +import { ExtensionManager } from '../../config/extension-manager.js'; +import { loadSettings } from '../../config/settings.js'; +import { promptForSetting } from '../../config/extensions/extensionSettings.js'; interface InstallArgs { path: string; @@ -26,10 +26,15 @@ export async function handleLink(args: InstallArgs) { source: args.path, type: 'link', }; - const extensionName = await installOrUpdateExtension( - installMetadata, - requestConsentNonInteractive, - ); + const workspaceDir = process.cwd(); + const extensionManager = new ExtensionManager({ + workspaceDir, + requestConsent: requestConsentNonInteractive, + requestSetting: promptForSetting, + loadedSettings: loadSettings(workspaceDir), + }); + const extensionName = + await extensionManager.installOrUpdateExtension(installMetadata); debugLogger.log( `Extension "${extensionName}" linked successfully and enabled.`, ); diff --git a/packages/cli/src/commands/extensions/list.ts b/packages/cli/src/commands/extensions/list.ts index 532d6b7233..432299c902 100644 --- a/packages/cli/src/commands/extensions/list.ts +++ b/packages/cli/src/commands/extensions/list.ts @@ -5,24 +5,32 @@ */ import type { CommandModule } from 'yargs'; -import { loadExtensions, toOutputString } from '../../config/extension.js'; import { getErrorMessage } from '../../utils/errors.js'; -import { ExtensionEnablementManager } from '../../config/extensions/extensionEnablement.js'; import { debugLogger } from '@google/gemini-cli-core'; +import { ExtensionManager } from '../../config/extension-manager.js'; +import { requestConsentNonInteractive } from '../../config/extensions/consent.js'; +import { loadSettings } from '../../config/settings.js'; +import { promptForSetting } from '../../config/extensions/extensionSettings.js'; export async function handleList() { try { - const extensions = loadExtensions( - new ExtensionEnablementManager(), - process.cwd(), - ); + const workspaceDir = process.cwd(); + const extensionManager = new ExtensionManager({ + workspaceDir, + requestConsent: requestConsentNonInteractive, + requestSetting: promptForSetting, + loadedSettings: loadSettings(workspaceDir), + }); + const extensions = extensionManager.loadExtensions(); if (extensions.length === 0) { debugLogger.log('No extensions installed.'); return; } debugLogger.log( extensions - .map((extension, _): string => toOutputString(extension, process.cwd())) + .map((extension, _): string => + extensionManager.toOutputString(extension), + ) .join('\n\n'), ); } catch (error) { diff --git a/packages/cli/src/commands/extensions/uninstall.ts b/packages/cli/src/commands/extensions/uninstall.ts index 82717f0f00..59dc8c828f 100644 --- a/packages/cli/src/commands/extensions/uninstall.ts +++ b/packages/cli/src/commands/extensions/uninstall.ts @@ -5,9 +5,12 @@ */ import type { CommandModule } from 'yargs'; -import { uninstallExtension } from '../../config/extension.js'; import { getErrorMessage } from '../../utils/errors.js'; import { debugLogger } from '@google/gemini-cli-core'; +import { requestConsentNonInteractive } from '../../config/extensions/consent.js'; +import { ExtensionManager } from '../../config/extension-manager.js'; +import { loadSettings } from '../../config/settings.js'; +import { promptForSetting } from '../../config/extensions/extensionSettings.js'; interface UninstallArgs { name: string; // can be extension name or source URL. @@ -15,7 +18,14 @@ interface UninstallArgs { export async function handleUninstall(args: UninstallArgs) { try { - await uninstallExtension(args.name, false); + const workspaceDir = process.cwd(); + const extensionManager = new ExtensionManager({ + workspaceDir, + requestConsent: requestConsentNonInteractive, + requestSetting: promptForSetting, + loadedSettings: loadSettings(workspaceDir), + }); + await extensionManager.uninstallExtension(args.name, false); debugLogger.log(`Extension "${args.name}" successfully uninstalled.`); } catch (error) { debugLogger.error(getErrorMessage(error)); diff --git a/packages/cli/src/commands/extensions/update.ts b/packages/cli/src/commands/extensions/update.ts index 39b9e174e3..5523149f18 100644 --- a/packages/cli/src/commands/extensions/update.ts +++ b/packages/cli/src/commands/extensions/update.ts @@ -5,10 +5,6 @@ */ import type { CommandModule } from 'yargs'; -import { - loadExtensions, - requestConsentNonInteractive, -} from '../../config/extension.js'; import { updateAllUpdatableExtensions, type ExtensionUpdateInfo, @@ -18,8 +14,11 @@ import { import { checkForExtensionUpdate } from '../../config/extensions/github.js'; import { getErrorMessage } from '../../utils/errors.js'; import { ExtensionUpdateState } from '../../ui/state/extensions.js'; -import { ExtensionEnablementManager } from '../../config/extensions/extensionEnablement.js'; import { debugLogger } from '@google/gemini-cli-core'; +import { ExtensionManager } from '../../config/extension-manager.js'; +import { requestConsentNonInteractive } from '../../config/extensions/consent.js'; +import { loadSettings } from '../../config/settings.js'; +import { promptForSetting } from '../../config/extensions/extensionSettings.js'; interface UpdateArgs { name?: string; @@ -30,13 +29,15 @@ const updateOutput = (info: ExtensionUpdateInfo) => `Extension "${info.name}" successfully updated: ${info.originalVersion} → ${info.updatedVersion}.`; export async function handleUpdate(args: UpdateArgs) { - const workingDir = process.cwd(); - const extensionEnablementManager = new ExtensionEnablementManager( - // Force enable named extensions, otherwise we will only update the enabled - // ones. - args.name ? [args.name] : [], - ); - const extensions = loadExtensions(extensionEnablementManager); + const workspaceDir = process.cwd(); + const extensionManager = new ExtensionManager({ + workspaceDir, + requestConsent: requestConsentNonInteractive, + requestSetting: promptForSetting, + loadedSettings: loadSettings(workspaceDir), + }); + + const extensions = extensionManager.loadExtensions(); if (args.name) { try { const extension = extensions.find( @@ -54,7 +55,7 @@ export async function handleUpdate(args: UpdateArgs) { } const updateState = await checkForExtensionUpdate( extension, - extensionEnablementManager, + extensionManager, ); if (updateState !== ExtensionUpdateState.UPDATE_AVAILABLE) { debugLogger.log(`Extension "${args.name}" is already up to date.`); @@ -63,9 +64,7 @@ export async function handleUpdate(args: UpdateArgs) { // TODO(chrstnb): we should list extensions if the requested extension is not installed. const updatedExtensionInfo = (await updateExtension( extension, - extensionEnablementManager, - workingDir, - requestConsentNonInteractive, + extensionManager, updateState, () => {}, ))!; @@ -88,7 +87,7 @@ export async function handleUpdate(args: UpdateArgs) { const extensionState = new Map(); await checkForAllExtensionUpdates( extensions, - extensionEnablementManager, + extensionManager, (action) => { if (action.type === 'SET_STATE') { extensionState.set(action.payload.name, { @@ -96,14 +95,11 @@ export async function handleUpdate(args: UpdateArgs) { }); } }, - workingDir, ); let updateInfos = await updateAllUpdatableExtensions( - workingDir, - requestConsentNonInteractive, extensions, extensionState, - extensionEnablementManager, + extensionManager, () => {}, ); updateInfos = updateInfos.filter( diff --git a/packages/cli/src/commands/mcp/list.test.ts b/packages/cli/src/commands/mcp/list.test.ts index 4c20d82c3f..ee9cf9395c 100644 --- a/packages/cli/src/commands/mcp/list.test.ts +++ b/packages/cli/src/commands/mcp/list.test.ts @@ -7,19 +7,20 @@ import { vi, describe, it, expect, beforeEach, type Mock } from 'vitest'; import { listMcpServers } from './list.js'; import { loadSettings } from '../../config/settings.js'; -import { ExtensionStorage, loadExtensions } from '../../config/extension.js'; import { createTransport, debugLogger } from '@google/gemini-cli-core'; import { Client } from '@modelcontextprotocol/sdk/client/index.js'; +import { ExtensionStorage } from '../../config/extensions/storage.js'; +import { ExtensionManager } from '../../config/extension-manager.js'; vi.mock('../../config/settings.js', () => ({ loadSettings: vi.fn(), })); -vi.mock('../../config/extension.js', () => ({ - loadExtensions: vi.fn(), +vi.mock('../../config/extensions/storage.js', () => ({ ExtensionStorage: { getUserExtensionsDir: vi.fn(), }, })); +vi.mock('../../config/extension-manager.js'); vi.mock('@google/gemini-cli-core', () => ({ createTransport: vi.fn(), MCPServerStatus: { @@ -46,9 +47,9 @@ vi.mock('@modelcontextprotocol/sdk/client/index.js'); const mockedGetUserExtensionsDir = ExtensionStorage.getUserExtensionsDir as Mock; const mockedLoadSettings = loadSettings as Mock; -const mockedLoadExtensions = loadExtensions as Mock; const mockedCreateTransport = createTransport as Mock; const MockedClient = Client as Mock; +const MockedExtensionManager = ExtensionManager as Mock; interface MockClient { connect: Mock; @@ -56,12 +57,17 @@ interface MockClient { close: Mock; } +interface MockExtensionManager { + loadExtensions: Mock; +} + interface MockTransport { close: Mock; } describe('mcp list command', () => { let mockClient: MockClient; + let mockExtensionManager: MockExtensionManager; let mockTransport: MockTransport; beforeEach(() => { @@ -73,10 +79,14 @@ describe('mcp list command', () => { ping: vi.fn(), close: vi.fn(), }; + mockExtensionManager = { + loadExtensions: vi.fn(), + }; MockedClient.mockImplementation(() => mockClient); + MockedExtensionManager.mockImplementation(() => mockExtensionManager); mockedCreateTransport.mockResolvedValue(mockTransport); - mockedLoadExtensions.mockReturnValue([]); + mockExtensionManager.loadExtensions.mockReturnValue([]); mockedGetUserExtensionsDir.mockReturnValue('/mocked/extensions/dir'); }); @@ -149,7 +159,7 @@ describe('mcp list command', () => { }, }); - mockedLoadExtensions.mockReturnValue([ + mockExtensionManager.loadExtensions.mockReturnValue([ { name: 'test-extension', mcpServers: { 'extension-server': { command: '/ext/server' } }, diff --git a/packages/cli/src/commands/mcp/list.ts b/packages/cli/src/commands/mcp/list.ts index 733f0071f8..3253641894 100644 --- a/packages/cli/src/commands/mcp/list.ts +++ b/packages/cli/src/commands/mcp/list.ts @@ -14,8 +14,9 @@ import { debugLogger, } from '@google/gemini-cli-core'; import { Client } from '@modelcontextprotocol/sdk/client/index.js'; -import { loadExtensions } from '../../config/extension.js'; -import { ExtensionEnablementManager } from '../../config/extensions/extensionEnablement.js'; +import { ExtensionManager } from '../../config/extension-manager.js'; +import { requestConsentNonInteractive } from '../../config/extensions/consent.js'; +import { promptForSetting } from '../../config/extensions/extensionSettings.js'; const COLOR_GREEN = '\u001b[32m'; const COLOR_YELLOW = '\u001b[33m'; @@ -26,7 +27,13 @@ async function getMcpServersFromConfig(): Promise< Record > { const settings = loadSettings(); - const extensions = loadExtensions(new ExtensionEnablementManager()); + const extensionManager = new ExtensionManager({ + loadedSettings: settings, + workspaceDir: process.cwd(), + requestConsent: requestConsentNonInteractive, + requestSetting: promptForSetting, + }); + const extensions = extensionManager.loadExtensions(); const mcpServers = { ...(settings.merged.mcpServers || {}) }; for (const extension of extensions) { Object.entries(extension.mcpServers || {}).forEach(([key, server]) => { diff --git a/packages/cli/src/config/extension-manager.ts b/packages/cli/src/config/extension-manager.ts new file mode 100644 index 0000000000..d175b8382c --- /dev/null +++ b/packages/cli/src/config/extension-manager.ts @@ -0,0 +1,643 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import * as os from 'node:os'; +import chalk from 'chalk'; +import { ExtensionEnablementManager } from './extensions/extensionEnablement.js'; +import { type LoadedSettings, SettingScope } from './settings.js'; +import { createHash, randomUUID } from 'node:crypto'; +import { loadInstallMetadata, type ExtensionConfig } from './extension.js'; +import { isWorkspaceTrusted } from './trustedFolders.js'; +import { + cloneFromGit, + downloadFromGitHubRelease, + tryParseGithubUrl, +} from './extensions/github.js'; +import { + Config, + debugLogger, + ExtensionDisableEvent, + ExtensionEnableEvent, + ExtensionInstallEvent, + ExtensionUninstallEvent, + ExtensionUpdateEvent, + getErrorMessage, + logExtensionDisable, + logExtensionEnable, + logExtensionInstallEvent, + logExtensionUninstall, + logExtensionUpdateEvent, + type MCPServerConfig, + type ExtensionInstallMetadata, + type GeminiCLIExtension, +} from '@google/gemini-cli-core'; +import { maybeRequestConsentOrFail } from './extensions/consent.js'; +import { resolveEnvVarsInObject } from '../utils/envVarResolver.js'; +import { ExtensionStorage } from './extensions/storage.js'; +import { + EXTENSIONS_CONFIG_FILENAME, + INSTALL_METADATA_FILENAME, + recursivelyHydrateStrings, + type JsonObject, +} from './extensions/variables.js'; +import { + getEnvContents, + maybePromptForSettings, + type ExtensionSetting, +} from './extensions/extensionSettings.js'; + +interface ExtensionManagerParams { + enabledExtensionOverrides?: string[]; + loadedSettings: LoadedSettings; + requestConsent: (consent: string) => Promise; + requestSetting: ((setting: ExtensionSetting) => Promise) | null; + workspaceDir: string; +} + +export class ExtensionManager { + private extensionEnablementManager: ExtensionEnablementManager; + private loadedSettings: LoadedSettings; + private requestConsent: (consent: string) => Promise; + private requestSetting: + | ((setting: ExtensionSetting) => Promise) + | null; + private telemetryConfig: Config; + private workspaceDir: string; + + constructor(options: ExtensionManagerParams) { + this.workspaceDir = options.workspaceDir; + this.extensionEnablementManager = new ExtensionEnablementManager( + options.enabledExtensionOverrides, + ); + this.loadedSettings = options.loadedSettings; + this.telemetryConfig = new Config({ + telemetry: options.loadedSettings.merged.telemetry, + interactive: false, + sessionId: randomUUID(), + targetDir: options.workspaceDir, + cwd: options.workspaceDir, + model: '', + debugMode: false, + }); + this.requestConsent = options.requestConsent; + this.requestSetting = options.requestSetting; + } + + async installOrUpdateExtension( + installMetadata: ExtensionInstallMetadata, + previousExtensionConfig?: ExtensionConfig, + ): Promise { + const isUpdate = !!previousExtensionConfig; + let newExtensionConfig: ExtensionConfig | null = null; + let localSourcePath: string | undefined; + try { + const settings = this.loadedSettings.merged; + if (!isWorkspaceTrusted(settings).isTrusted) { + throw new Error( + `Could not install extension from untrusted folder at ${installMetadata.source}`, + ); + } + + const extensionsDir = ExtensionStorage.getUserExtensionsDir(); + await fs.promises.mkdir(extensionsDir, { recursive: true }); + + if ( + !path.isAbsolute(installMetadata.source) && + (installMetadata.type === 'local' || installMetadata.type === 'link') + ) { + installMetadata.source = path.resolve( + this.workspaceDir, + installMetadata.source, + ); + } + + let tempDir: string | undefined; + + if ( + installMetadata.type === 'git' || + installMetadata.type === 'github-release' + ) { + tempDir = await ExtensionStorage.createTmpDir(); + const parsedGithubParts = tryParseGithubUrl(installMetadata.source); + if (!parsedGithubParts) { + await cloneFromGit(installMetadata, tempDir); + installMetadata.type = 'git'; + } else { + const result = await downloadFromGitHubRelease( + installMetadata, + tempDir, + parsedGithubParts, + ); + if (result.success) { + installMetadata.type = result.type; + installMetadata.releaseTag = result.tagName; + } else if ( + // This repo has no github releases, and wasn't explicitly installed + // from a github release, unconditionally just clone it. + (result.failureReason === 'no release data' && + installMetadata.type === 'git') || + // Otherwise ask the user if they would like to try a git clone. + (await this.requestConsent( + `Error downloading github release for ${installMetadata.source} with the following error: ${result.errorMessage}.\n\nWould you like to attempt to install via "git clone" instead?`, + )) + ) { + await cloneFromGit(installMetadata, tempDir); + installMetadata.type = 'git'; + } else { + throw new Error( + `Failed to install extension ${installMetadata.source}: ${result.errorMessage}`, + ); + } + } + localSourcePath = tempDir; + } else if ( + installMetadata.type === 'local' || + installMetadata.type === 'link' + ) { + localSourcePath = installMetadata.source; + } else { + throw new Error(`Unsupported install type: ${installMetadata.type}`); + } + + try { + newExtensionConfig = this.loadExtensionConfig(localSourcePath); + + if (isUpdate && installMetadata.autoUpdate) { + const oldSettings = new Set( + previousExtensionConfig.settings?.map((s) => s.name) || [], + ); + const newSettings = new Set( + newExtensionConfig.settings?.map((s) => s.name) || [], + ); + + const settingsAreEqual = + oldSettings.size === newSettings.size && + [...oldSettings].every((value) => newSettings.has(value)); + + if (!settingsAreEqual && installMetadata.autoUpdate) { + throw new Error( + `Extension "${newExtensionConfig.name}" has settings changes and cannot be auto-updated. Please update manually.`, + ); + } + } + + const newExtensionName = newExtensionConfig.name; + if (!isUpdate) { + const installedExtensions = this.loadExtensions(); + if ( + installedExtensions.some( + (installed) => installed.name === newExtensionName, + ) + ) { + throw new Error( + `Extension "${newExtensionName}" is already installed. Please uninstall it first.`, + ); + } + } + + await maybeRequestConsentOrFail( + newExtensionConfig, + this.requestConsent, + previousExtensionConfig, + ); + + const extensionStorage = new ExtensionStorage(newExtensionName); + const destinationPath = extensionStorage.getExtensionDir(); + let previousSettings: Record | undefined; + if (isUpdate) { + previousSettings = getEnvContents(extensionStorage); + await this.uninstallExtension(newExtensionName, isUpdate); + } + + await fs.promises.mkdir(destinationPath, { recursive: true }); + if (this.requestSetting) { + if (isUpdate) { + await maybePromptForSettings( + newExtensionConfig, + this.requestSetting, + previousExtensionConfig, + previousSettings, + ); + } else { + await maybePromptForSettings( + newExtensionConfig, + this.requestSetting, + ); + } + } + + if ( + installMetadata.type === 'local' || + installMetadata.type === 'git' || + installMetadata.type === 'github-release' + ) { + await copyExtension(localSourcePath, destinationPath); + } + + const metadataString = JSON.stringify(installMetadata, null, 2); + const metadataPath = path.join( + destinationPath, + INSTALL_METADATA_FILENAME, + ); + await fs.promises.writeFile(metadataPath, metadataString); + } finally { + if (tempDir) { + await fs.promises.rm(tempDir, { recursive: true, force: true }); + } + } + + if (isUpdate) { + logExtensionUpdateEvent( + this.telemetryConfig, + new ExtensionUpdateEvent( + hashValue(newExtensionConfig.name), + getExtensionId(newExtensionConfig, installMetadata), + newExtensionConfig.version, + previousExtensionConfig.version, + installMetadata.type, + 'success', + ), + ); + } else { + logExtensionInstallEvent( + this.telemetryConfig, + new ExtensionInstallEvent( + hashValue(newExtensionConfig.name), + getExtensionId(newExtensionConfig, installMetadata), + newExtensionConfig.version, + installMetadata.type, + 'success', + ), + ); + this.enableExtension(newExtensionConfig.name, SettingScope.User); + } + + return newExtensionConfig!.name; + } catch (error) { + // Attempt to load config from the source path even if installation fails + // to get the name and version for logging. + if (!newExtensionConfig && localSourcePath) { + try { + newExtensionConfig = this.loadExtensionConfig(localSourcePath); + } catch { + // Ignore error, this is just for logging. + } + } + const config = newExtensionConfig ?? previousExtensionConfig; + const extensionId = config + ? getExtensionId(config, installMetadata) + : undefined; + if (isUpdate) { + logExtensionUpdateEvent( + this.telemetryConfig, + new ExtensionUpdateEvent( + hashValue(config?.name ?? ''), + extensionId ?? '', + newExtensionConfig?.version ?? '', + previousExtensionConfig.version, + installMetadata.type, + 'error', + ), + ); + } else { + logExtensionInstallEvent( + this.telemetryConfig, + new ExtensionInstallEvent( + hashValue(newExtensionConfig?.name ?? ''), + extensionId ?? '', + newExtensionConfig?.version ?? '', + installMetadata.type, + 'error', + ), + ); + } + throw error; + } + } + + async uninstallExtension( + extensionIdentifier: string, + isUpdate: boolean, + ): Promise { + const installedExtensions = this.loadExtensions(); + const extension = installedExtensions.find( + (installed) => + installed.name.toLowerCase() === extensionIdentifier.toLowerCase() || + installed.installMetadata?.source.toLowerCase() === + extensionIdentifier.toLowerCase(), + ); + if (!extension) { + throw new Error(`Extension not found.`); + } + const storage = new ExtensionStorage(extension.name); + + await fs.promises.rm(storage.getExtensionDir(), { + recursive: true, + force: true, + }); + + // The rest of the cleanup below here is only for true uninstalls, not + // uninstalls related to updates. + if (isUpdate) return; + + this.extensionEnablementManager.remove(extension.name); + + logExtensionUninstall( + this.telemetryConfig, + new ExtensionUninstallEvent( + hashValue(extension.name), + extension.id, + 'success', + ), + ); + } + + loadExtensions(): GeminiCLIExtension[] { + const extensionsDir = ExtensionStorage.getUserExtensionsDir(); + if (!fs.existsSync(extensionsDir)) { + return []; + } + + const extensions: GeminiCLIExtension[] = []; + for (const subdir of fs.readdirSync(extensionsDir)) { + const extensionDir = path.join(extensionsDir, subdir); + + const extension = this.loadExtension(extensionDir); + if (extension != null) { + extensions.push(extension); + } + } + + const uniqueExtensions = new Map(); + + for (const extension of extensions) { + if (!uniqueExtensions.has(extension.name)) { + uniqueExtensions.set(extension.name, extension); + } + } + + return Array.from(uniqueExtensions.values()); + } + + loadExtension(extensionDir: string): GeminiCLIExtension | null { + if (!fs.statSync(extensionDir).isDirectory()) { + return null; + } + + const installMetadata = loadInstallMetadata(extensionDir); + let effectiveExtensionPath = extensionDir; + + if (installMetadata?.type === 'link') { + effectiveExtensionPath = installMetadata.source; + } + + try { + let config = this.loadExtensionConfig(effectiveExtensionPath); + + const customEnv = getEnvContents(new ExtensionStorage(config.name)); + config = resolveEnvVarsInObject(config, customEnv); + + if (config.mcpServers) { + config.mcpServers = Object.fromEntries( + Object.entries(config.mcpServers).map(([key, value]) => [ + key, + filterMcpConfig(value), + ]), + ); + } + + const contextFiles = getContextFileNames(config) + .map((contextFileName) => + path.join(effectiveExtensionPath, contextFileName), + ) + .filter((contextFilePath) => fs.existsSync(contextFilePath)); + + return { + name: config.name, + version: config.version, + path: effectiveExtensionPath, + contextFiles, + installMetadata, + mcpServers: config.mcpServers, + excludeTools: config.excludeTools, + isActive: this.extensionEnablementManager.isEnabled( + config.name, + this.workspaceDir, + ), + id: getExtensionId(config, installMetadata), + }; + } catch (e) { + debugLogger.error( + `Warning: Skipping extension in ${effectiveExtensionPath}: ${getErrorMessage( + e, + )}`, + ); + return null; + } + } + + loadExtensionByName(name: string): GeminiCLIExtension | null { + const userExtensionsDir = ExtensionStorage.getUserExtensionsDir(); + if (!fs.existsSync(userExtensionsDir)) { + return null; + } + + for (const subdir of fs.readdirSync(userExtensionsDir)) { + const extensionDir = path.join(userExtensionsDir, subdir); + if (!fs.statSync(extensionDir).isDirectory()) { + continue; + } + const extension = this.loadExtension(extensionDir); + if (extension && extension.name.toLowerCase() === name.toLowerCase()) { + return extension; + } + } + + return null; + } + + loadExtensionConfig(extensionDir: string): ExtensionConfig { + const configFilePath = path.join(extensionDir, EXTENSIONS_CONFIG_FILENAME); + if (!fs.existsSync(configFilePath)) { + throw new Error(`Configuration file not found at ${configFilePath}`); + } + try { + const configContent = fs.readFileSync(configFilePath, 'utf-8'); + const rawConfig = JSON.parse(configContent) as ExtensionConfig; + if (!rawConfig.name || !rawConfig.version) { + throw new Error( + `Invalid configuration in ${configFilePath}: missing ${!rawConfig.name ? '"name"' : '"version"'}`, + ); + } + const installDir = new ExtensionStorage(rawConfig.name).getExtensionDir(); + const config = recursivelyHydrateStrings( + rawConfig as unknown as JsonObject, + { + extensionPath: installDir, + workspacePath: this.workspaceDir, + '/': path.sep, + pathSeparator: path.sep, + }, + ) as unknown as ExtensionConfig; + + validateName(config.name); + return config; + } catch (e) { + throw new Error( + `Failed to load extension config from ${configFilePath}: ${getErrorMessage( + e, + )}`, + ); + } + } + + toOutputString(extension: GeminiCLIExtension): string { + const userEnabled = this.extensionEnablementManager.isEnabled( + extension.name, + os.homedir(), + ); + const workspaceEnabled = this.extensionEnablementManager.isEnabled( + extension.name, + this.workspaceDir, + ); + + const status = workspaceEnabled ? chalk.green('✓') : chalk.red('✗'); + let output = `${status} ${extension.name} (${extension.version})`; + output += `\n ID: ${extension.id}`; + output += `\n Path: ${extension.path}`; + if (extension.installMetadata) { + output += `\n Source: ${extension.installMetadata.source} (Type: ${extension.installMetadata.type})`; + if (extension.installMetadata.ref) { + output += `\n Ref: ${extension.installMetadata.ref}`; + } + if (extension.installMetadata.releaseTag) { + output += `\n Release tag: ${extension.installMetadata.releaseTag}`; + } + } + output += `\n Enabled (User): ${userEnabled}`; + output += `\n Enabled (Workspace): ${workspaceEnabled}`; + if (extension.contextFiles.length > 0) { + output += `\n Context files:`; + extension.contextFiles.forEach((contextFile) => { + output += `\n ${contextFile}`; + }); + } + if (extension.mcpServers) { + output += `\n MCP servers:`; + Object.keys(extension.mcpServers).forEach((key) => { + output += `\n ${key}`; + }); + } + if (extension.excludeTools) { + output += `\n Excluded tools:`; + extension.excludeTools.forEach((tool) => { + output += `\n ${tool}`; + }); + } + return output; + } + + disableExtension(name: string, scope: SettingScope) { + if ( + scope === SettingScope.System || + scope === SettingScope.SystemDefaults + ) { + throw new Error('System and SystemDefaults scopes are not supported.'); + } + const extension = this.loadExtensionByName(name); + if (!extension) { + throw new Error(`Extension with name ${name} does not exist.`); + } + + const scopePath = + scope === SettingScope.Workspace ? this.workspaceDir : os.homedir(); + this.extensionEnablementManager.disable(name, true, scopePath); + logExtensionDisable( + this.telemetryConfig, + new ExtensionDisableEvent(hashValue(name), extension.id, scope), + ); + } + + enableExtension(name: string, scope: SettingScope) { + if ( + scope === SettingScope.System || + scope === SettingScope.SystemDefaults + ) { + throw new Error('System and SystemDefaults scopes are not supported.'); + } + const extension = this.loadExtensionByName(name); + if (!extension) { + throw new Error(`Extension with name ${name} does not exist.`); + } + const scopePath = + scope === SettingScope.Workspace ? this.workspaceDir : os.homedir(); + this.extensionEnablementManager.enable(name, true, scopePath); + logExtensionEnable( + this.telemetryConfig, + new ExtensionEnableEvent(hashValue(name), extension.id, scope), + ); + } +} + +function filterMcpConfig(original: MCPServerConfig): MCPServerConfig { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { trust, ...rest } = original; + return Object.freeze(rest); +} + +export async function copyExtension( + source: string, + destination: string, +): Promise { + await fs.promises.cp(source, destination, { recursive: true }); +} + +function getContextFileNames(config: ExtensionConfig): string[] { + if (!config.contextFileName) { + return ['GEMINI.md']; + } else if (!Array.isArray(config.contextFileName)) { + return [config.contextFileName]; + } + return config.contextFileName; +} + +function validateName(name: string) { + if (!/^[a-zA-Z0-9-]+$/.test(name)) { + throw new Error( + `Invalid extension name: "${name}". Only letters (a-z, A-Z), numbers (0-9), and dashes (-) are allowed.`, + ); + } +} + +export function getExtensionId( + config: ExtensionConfig, + installMetadata?: ExtensionInstallMetadata, +): string { + // IDs are created by hashing details of the installation source in order to + // deduplicate extensions with conflicting names and also obfuscate any + // potentially sensitive information such as private git urls, system paths, + // or project names. + let idValue = config.name; + const githubUrlParts = + installMetadata && + (installMetadata.type === 'git' || + installMetadata.type === 'github-release') + ? tryParseGithubUrl(installMetadata.source) + : null; + if (githubUrlParts) { + // For github repos, we use the https URI to the repo as the ID. + idValue = `https://github.com/${githubUrlParts.owner}/${githubUrlParts.repo}`; + } else { + idValue = installMetadata?.source ?? config.name; + } + return hashValue(idValue); +} + +export function hashValue(value: string): string { + return createHash('sha256').update(value).digest('hex'); +} diff --git a/packages/cli/src/config/extension.test.ts b/packages/cli/src/config/extension.test.ts index 7bc6bd116c..9d81a26be2 100644 --- a/packages/cli/src/config/extension.test.ts +++ b/packages/cli/src/config/extension.test.ts @@ -4,37 +4,29 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { vi } from 'vitest'; +import { vi, type MockedFunction } from 'vitest'; import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; -import { createHash } from 'node:crypto'; import { - EXTENSIONS_CONFIG_FILENAME, - ExtensionStorage, - INSTALL_METADATA_FILENAME, - INSTALL_WARNING_MESSAGE, - disableExtension, - enableExtension, - installOrUpdateExtension, - loadExtension, - loadExtensionConfig, - loadExtensions, - uninstallExtension, - hashValue, -} from './extension.js'; -import { - GEMINI_DIR, type GeminiCLIExtension, ExtensionUninstallEvent, ExtensionDisableEvent, ExtensionEnableEvent, } from '@google/gemini-cli-core'; -import { SettingScope } from './settings.js'; +import { loadSettings, SettingScope } from './settings.js'; import { isWorkspaceTrusted } from './trustedFolders.js'; import { createExtension } from '../test-utils/createExtension.js'; import { ExtensionEnablementManager } from './extensions/extensionEnablement.js'; import { join } from 'node:path'; +import { + EXTENSIONS_CONFIG_FILENAME, + EXTENSIONS_DIRECTORY_NAME, + INSTALL_METADATA_FILENAME, +} from './extensions/variables.js'; +import { hashValue, ExtensionManager } from './extension-manager.js'; +import { ExtensionStorage } from './extensions/storage.js'; +import { INSTALL_WARNING_MESSAGE } from './extensions/consent.js'; import type { ExtensionSetting } from './extensions/extensionSettings.js'; const mockGit = { @@ -113,12 +105,15 @@ vi.mock('child_process', async (importOriginal) => { }; }); -const EXTENSIONS_DIRECTORY_NAME = path.join(GEMINI_DIR, 'extensions'); - describe('extension tests', () => { let tempHomeDir: string; let tempWorkspaceDir: string; let userExtensionsDir: string; + let extensionManager: ExtensionManager; + let mockRequestConsent: MockedFunction<(consent: string) => Promise>; + let mockPromptForSettings: MockedFunction< + (setting: ExtensionSetting) => Promise + >; beforeEach(() => { tempHomeDir = fs.mkdtempSync( @@ -128,14 +123,23 @@ describe('extension tests', () => { path.join(tempHomeDir, 'gemini-cli-test-workspace-'), ); userExtensionsDir = path.join(tempHomeDir, EXTENSIONS_DIRECTORY_NAME); + mockRequestConsent = vi.fn(); + mockRequestConsent.mockResolvedValue(true); + mockPromptForSettings = vi.fn(); + mockPromptForSettings.mockResolvedValue(''); fs.mkdirSync(userExtensionsDir, { recursive: true }); - vi.mocked(os.homedir).mockReturnValue(tempHomeDir); vi.mocked(isWorkspaceTrusted).mockReturnValue({ isTrusted: true, source: undefined, }); vi.spyOn(process, 'cwd').mockReturnValue(tempWorkspaceDir); + extensionManager = new ExtensionManager({ + workspaceDir: tempWorkspaceDir, + requestConsent: mockRequestConsent, + requestSetting: mockPromptForSettings, + loadedSettings: loadSettings(tempWorkspaceDir), + }); }); afterEach(() => { @@ -155,7 +159,7 @@ describe('extension tests', () => { version: '1.0.0', }); - const extensions = loadExtensions(new ExtensionEnablementManager()); + const extensions = extensionManager.loadExtensions(); expect(extensions).toHaveLength(1); expect(extensions[0].path).toBe(extensionDir); expect(extensions[0].name).toBe('test-extension'); @@ -174,7 +178,7 @@ describe('extension tests', () => { version: '2.0.0', }); - const extensions = loadExtensions(new ExtensionEnablementManager()); + const extensions = extensionManager.loadExtensions(); expect(extensions).toHaveLength(2); const ext1 = extensions.find((e) => e.name === 'ext1'); @@ -194,7 +198,7 @@ describe('extension tests', () => { contextFileName: 'my-context-file.md', }); - const extensions = loadExtensions(new ExtensionEnablementManager()); + const extensions = extensionManager.loadExtensions(); expect(extensions).toHaveLength(1); const ext1 = extensions.find((e) => e.name === 'ext1'); @@ -214,14 +218,11 @@ describe('extension tests', () => { name: 'enabled-extension', version: '2.0.0', }); - const manager = new ExtensionEnablementManager(); - disableExtension( + extensionManager.disableExtension( 'disabled-extension', SettingScope.User, - manager, - tempWorkspaceDir, ); - const extensions = loadExtensions(manager); + const extensions = extensionManager.loadExtensions(); expect(extensions).toHaveLength(2); expect(extensions[0].name).toBe('disabled-extension'); expect(extensions[0].isActive).toBe(false); @@ -243,7 +244,7 @@ describe('extension tests', () => { }, }); - const extensions = loadExtensions(new ExtensionEnablementManager()); + const extensions = extensionManager.loadExtensions(); expect(extensions).toHaveLength(1); const expectedCwd = path.join( userExtensionsDir, @@ -262,16 +263,13 @@ describe('extension tests', () => { }); fs.writeFileSync(path.join(sourceExtDir, 'context.md'), 'linked context'); - const extensionName = await installOrUpdateExtension( - { - source: sourceExtDir, - type: 'link', - }, - async (_) => true, - ); + const extensionName = await extensionManager.installOrUpdateExtension({ + source: sourceExtDir, + type: 'link', + }); expect(extensionName).toEqual('my-linked-extension'); - const extensions = loadExtensions(new ExtensionEnablementManager()); + const extensions = extensionManager.loadExtensions(); expect(extensions).toHaveLength(1); const linkedExt = extensions[0]; @@ -320,7 +318,7 @@ describe('extension tests', () => { }; fs.writeFileSync(configPath, JSON.stringify(extensionConfig)); - const extensions = loadExtensions(new ExtensionEnablementManager()); + const extensions = extensionManager.loadExtensions(); expect(extensions).toHaveLength(1); const extension = extensions[0]; @@ -361,7 +359,7 @@ describe('extension tests', () => { const envFilePath = path.join(extDir, '.env'); fs.writeFileSync(envFilePath, 'MY_API_KEY=test-key-from-file\n'); - const extensions = loadExtensions(new ExtensionEnablementManager()); + const extensions = extensionManager.loadExtensions(); expect(extensions).toHaveLength(1); const extension = extensions[0]; @@ -401,7 +399,7 @@ describe('extension tests', () => { JSON.stringify(extensionConfig), ); - const extensions = loadExtensions(new ExtensionEnablementManager()); + const extensions = extensionManager.loadExtensions(); expect(extensions).toHaveLength(1); const extension = extensions[0]; @@ -429,7 +427,7 @@ describe('extension tests', () => { const badConfigPath = path.join(badExtDir, EXTENSIONS_CONFIG_FILENAME); fs.writeFileSync(badConfigPath, '{ "name": "bad-ext"'); // Malformed - const extensions = loadExtensions(new ExtensionEnablementManager()); + const extensions = extensionManager.loadExtensions(); expect(extensions).toHaveLength(1); expect(extensions[0].name).toBe('good-ext'); @@ -461,7 +459,7 @@ describe('extension tests', () => { const badConfigPath = path.join(badExtDir, EXTENSIONS_CONFIG_FILENAME); fs.writeFileSync(badConfigPath, JSON.stringify({ version: '1.0.0' })); - const extensions = loadExtensions(new ExtensionEnablementManager()); + const extensions = extensionManager.loadExtensions(); expect(extensions).toHaveLength(1); expect(extensions[0].name).toBe('good-ext'); @@ -489,7 +487,7 @@ describe('extension tests', () => { }, }); - const extensions = loadExtensions(new ExtensionEnablementManager()); + const extensions = extensionManager.loadExtensions(); expect(extensions).toHaveLength(1); expect(extensions[0].mcpServers?.['test-server'].trust).toBeUndefined(); }); @@ -504,11 +502,7 @@ describe('extension tests', () => { version: '1.0.0', }); - const extension = loadExtension({ - extensionDir: badExtDir, - workspaceDir: tempWorkspaceDir, - extensionEnablementManager: new ExtensionEnablementManager(), - }); + const extension = extensionManager.loadExtension(badExtDir); expect(extension).toBeNull(); expect(consoleSpy).toHaveBeenCalledWith( @@ -529,16 +523,8 @@ describe('extension tests', () => { }, }); - const extension = loadExtension({ - extensionDir, - workspaceDir: tempWorkspaceDir, - extensionEnablementManager: new ExtensionEnablementManager(), - }); - - const expectedHash = createHash('sha256') - .update('http://somehost.com/foo/bar') - .digest('hex'); - expect(extension?.id).toBe(expectedHash); + const extension = extensionManager.loadExtension(extensionDir); + expect(extension?.id).toBe(hashValue('http://somehost.com/foo/bar')); }); it('should generate id from owner/repo for github http urls', () => { @@ -552,16 +538,8 @@ describe('extension tests', () => { }, }); - const extension = loadExtension({ - extensionDir, - workspaceDir: tempWorkspaceDir, - extensionEnablementManager: new ExtensionEnablementManager(), - }); - - const expectedHash = createHash('sha256') - .update('https://github.com/foo/bar') - .digest('hex'); - expect(extension?.id).toBe(expectedHash); + const extension = extensionManager.loadExtension(extensionDir); + expect(extension?.id).toBe(hashValue('https://github.com/foo/bar')); }); it('should generate id from owner/repo for github ssh urls', () => { @@ -575,16 +553,8 @@ describe('extension tests', () => { }, }); - const extension = loadExtension({ - extensionDir, - workspaceDir: tempWorkspaceDir, - extensionEnablementManager: new ExtensionEnablementManager(), - }); - - const expectedHash = createHash('sha256') - .update('https://github.com/foo/bar') - .digest('hex'); - expect(extension?.id).toBe(expectedHash); + const extension = extensionManager.loadExtension(extensionDir); + expect(extension?.id).toBe(hashValue('https://github.com/foo/bar')); }); it('should generate id from source for github-release extension', () => { @@ -598,16 +568,8 @@ describe('extension tests', () => { }, }); - const extension = loadExtension({ - extensionDir, - workspaceDir: tempWorkspaceDir, - extensionEnablementManager: new ExtensionEnablementManager(), - }); - - const expectedHash = createHash('sha256') - .update('https://github.com/foo/bar') - .digest('hex'); - expect(extension?.id).toBe(expectedHash); + const extension = extensionManager.loadExtension(extensionDir); + expect(extension?.id).toBe(hashValue('https://github.com/foo/bar')); }); it('should generate id from the original source for local extension', () => { @@ -621,16 +583,8 @@ describe('extension tests', () => { }, }); - const extension = loadExtension({ - extensionDir, - workspaceDir: tempWorkspaceDir, - extensionEnablementManager: new ExtensionEnablementManager(), - }); - - const expectedHash = createHash('sha256') - .update('/some/path') - .digest('hex'); - expect(extension?.id).toBe(expectedHash); + const extension = extensionManager.loadExtension(extensionDir); + expect(extension?.id).toBe(hashValue('/some/path')); }); it('should generate id from the original source for linked extensions', async () => { @@ -640,25 +594,15 @@ describe('extension tests', () => { name: 'link-ext-name', version: '1.0.0', }); - const extensionName = await installOrUpdateExtension( - { - type: 'link', - source: actualExtensionDir, - }, - async () => true, - tempWorkspaceDir, - ); - - const extension = loadExtension({ - extensionDir: new ExtensionStorage(extensionName).getExtensionDir(), - workspaceDir: tempWorkspaceDir, - extensionEnablementManager: new ExtensionEnablementManager(), + const extensionName = await extensionManager.installOrUpdateExtension({ + type: 'link', + source: actualExtensionDir, }); - const expectedHash = createHash('sha256') - .update(actualExtensionDir) - .digest('hex'); - expect(extension?.id).toBe(expectedHash); + const extension = extensionManager.loadExtension( + new ExtensionStorage(extensionName).getExtensionDir(), + ); + expect(extension?.id).toBe(hashValue(actualExtensionDir)); }); it('should generate id from name for extension with no install metadata', () => { @@ -668,16 +612,8 @@ describe('extension tests', () => { version: '1.0.0', }); - const extension = loadExtension({ - extensionDir, - workspaceDir: tempWorkspaceDir, - extensionEnablementManager: new ExtensionEnablementManager(), - }); - - const expectedHash = createHash('sha256') - .update('no-meta-name') - .digest('hex'); - expect(extension?.id).toBe(expectedHash); + const extension = extensionManager.loadExtension(extensionDir); + expect(extension?.id).toBe(hashValue('no-meta-name')); }); }); }); @@ -692,10 +628,10 @@ describe('extension tests', () => { const targetExtDir = path.join(userExtensionsDir, 'my-local-extension'); const metadataPath = path.join(targetExtDir, INSTALL_METADATA_FILENAME); - await installOrUpdateExtension( - { source: sourceExtDir, type: 'local' }, - async (_) => true, - ); + await extensionManager.installOrUpdateExtension({ + source: sourceExtDir, + type: 'local', + }); expect(fs.existsSync(targetExtDir)).toBe(true); expect(fs.existsSync(metadataPath)).toBe(true); @@ -713,15 +649,15 @@ describe('extension tests', () => { name: 'my-local-extension', version: '1.0.0', }); - await installOrUpdateExtension( - { source: sourceExtDir, type: 'local' }, - async (_) => true, - ); + await extensionManager.installOrUpdateExtension({ + source: sourceExtDir, + type: 'local', + }); await expect( - installOrUpdateExtension( - { source: sourceExtDir, type: 'local' }, - async (_) => true, - ), + extensionManager.installOrUpdateExtension({ + source: sourceExtDir, + type: 'local', + }), ).rejects.toThrow( 'Extension "my-local-extension" is already installed. Please uninstall it first.', ); @@ -733,10 +669,10 @@ describe('extension tests', () => { const configPath = path.join(sourceExtDir, EXTENSIONS_CONFIG_FILENAME); await expect( - installOrUpdateExtension( - { source: sourceExtDir, type: 'local' }, - async (_) => true, - ), + extensionManager.installOrUpdateExtension({ + source: sourceExtDir, + type: 'local', + }), ).rejects.toThrow(`Configuration file not found at ${configPath}`); const targetExtDir = path.join(userExtensionsDir, 'bad-extension'); @@ -750,10 +686,10 @@ describe('extension tests', () => { fs.writeFileSync(configPath, '{ "name": "bad-json", "version": "1.0.0"'); // Malformed JSON await expect( - installOrUpdateExtension( - { source: sourceExtDir, type: 'local' }, - async (_) => true, - ), + extensionManager.installOrUpdateExtension({ + source: sourceExtDir, + type: 'local', + }), ).rejects.toThrow( new RegExp( `^Failed to load extension config from ${configPath.replace( @@ -775,10 +711,10 @@ describe('extension tests', () => { fs.writeFileSync(configPath, JSON.stringify({ version: '1.0.0' })); await expect( - installOrUpdateExtension( - { source: sourceExtDir, type: 'local' }, - async (_) => true, - ), + extensionManager.installOrUpdateExtension({ + source: sourceExtDir, + type: 'local', + }), ).rejects.toThrow( `Invalid configuration in ${configPath}: missing "name"`, ); @@ -806,10 +742,10 @@ describe('extension tests', () => { type: 'github-release', }); - await installOrUpdateExtension( - { source: gitUrl, type: 'git' }, - async (_) => true, - ); + await extensionManager.installOrUpdateExtension({ + source: gitUrl, + type: 'git', + }); expect(fs.existsSync(targetExtDir)).toBe(true); expect(fs.existsSync(metadataPath)).toBe(true); @@ -830,10 +766,10 @@ describe('extension tests', () => { const metadataPath = path.join(targetExtDir, INSTALL_METADATA_FILENAME); const configPath = path.join(targetExtDir, EXTENSIONS_CONFIG_FILENAME); - await installOrUpdateExtension( - { source: sourceExtDir, type: 'link' }, - async (_) => true, - ); + await extensionManager.installOrUpdateExtension({ + source: sourceExtDir, + type: 'link', + }); expect(fs.existsSync(targetExtDir)).toBe(true); expect(fs.existsSync(metadataPath)).toBe(true); @@ -860,20 +796,18 @@ describe('extension tests', () => { version: '1.1.0', }); if (isUpdate) { - await installOrUpdateExtension( - { source: sourceExtDir, type: 'local' }, - async (_) => true, - ); + await extensionManager.installOrUpdateExtension({ + source: sourceExtDir, + type: 'local', + }); } // Clears out any calls to mocks from the above function calls. vi.clearAllMocks(); }); it(`should log an ${isUpdate ? 'update' : 'install'} event to clearcut on success`, async () => { - await installOrUpdateExtension( + await extensionManager.installOrUpdateExtension( { source: sourceExtDir, type: 'local' }, - async (_) => true, - undefined, isUpdate ? { name: 'my-local-extension', @@ -895,10 +829,8 @@ describe('extension tests', () => { const enablementManager = new ExtensionEnablementManager(); enablementManager.enable('my-local-extension', true, '/some/scope'); - await installOrUpdateExtension( + await extensionManager.installOrUpdateExtension( { source: sourceExtDir, type: 'local' }, - async (_) => true, - undefined, isUpdate ? { name: 'my-local-extension', @@ -936,14 +868,11 @@ describe('extension tests', () => { }, }); - const mockRequestConsent = vi.fn(); - mockRequestConsent.mockResolvedValue(true); - await expect( - installOrUpdateExtension( - { source: sourceExtDir, type: 'local' }, - mockRequestConsent, - ), + extensionManager.installOrUpdateExtension({ + source: sourceExtDir, + type: 'local', + }), ).resolves.toBe('my-local-extension'); expect(mockRequestConsent).toHaveBeenCalledWith( @@ -969,10 +898,10 @@ This extension will run the following MCP servers: }); await expect( - installOrUpdateExtension( - { source: sourceExtDir, type: 'local' }, - async () => true, - ), + extensionManager.installOrUpdateExtension({ + source: sourceExtDir, + type: 'local', + }), ).resolves.toBe('my-local-extension'); }); @@ -988,12 +917,12 @@ This extension will run the following MCP servers: }, }, }); - + mockRequestConsent.mockResolvedValue(false); await expect( - installOrUpdateExtension( - { source: sourceExtDir, type: 'local' }, - async () => false, - ), + extensionManager.installOrUpdateExtension({ + source: sourceExtDir, + type: 'local', + }), ).rejects.toThrow('Installation cancelled for "my-local-extension".'); }); @@ -1006,14 +935,11 @@ This extension will run the following MCP servers: const targetExtDir = path.join(userExtensionsDir, 'my-local-extension'); const metadataPath = path.join(targetExtDir, INSTALL_METADATA_FILENAME); - await installOrUpdateExtension( - { - source: sourceExtDir, - type: 'local', - autoUpdate: true, - }, - async (_) => true, - ); + await extensionManager.installOrUpdateExtension({ + source: sourceExtDir, + type: 'local', + autoUpdate: true, + }); expect(fs.existsSync(targetExtDir)).toBe(true); expect(fs.existsSync(metadataPath)).toBe(true); @@ -1039,29 +965,24 @@ This extension will run the following MCP servers: }, }); - const mockRequestConsent = vi.fn(); - // Install it and force consent first. - await installOrUpdateExtension( - { source: sourceExtDir, type: 'local' }, - async () => true, - ); + // Install it with hard coded consent first. + await extensionManager.installOrUpdateExtension({ + source: sourceExtDir, + type: 'local', + }); + expect(mockRequestConsent).toHaveBeenCalledOnce(); // Now update it without changing anything. await expect( - installOrUpdateExtension( + extensionManager.installOrUpdateExtension( { source: sourceExtDir, type: 'local' }, - mockRequestConsent, - process.cwd(), // Provide its own existing config as the previous config. - await loadExtensionConfig({ - extensionDir: sourceExtDir, - workspaceDir: process.cwd(), - extensionEnablementManager: new ExtensionEnablementManager(), - }), + await extensionManager.loadExtensionConfig(sourceExtDir), ), ).resolves.toBe('my-local-extension'); - expect(mockRequestConsent).not.toHaveBeenCalled(); + // Still only called once + expect(mockRequestConsent).toHaveBeenCalledOnce(); }); it('should prompt for settings if promptForSettings', async () => { @@ -1078,18 +999,12 @@ This extension will run the following MCP servers: ], }); - const promptForSettingsMock = vi.fn( - async (_: ExtensionSetting): Promise => Promise.resolve(''), - ); - await installOrUpdateExtension( - { source: sourceExtDir, type: 'local' }, - async (_) => true, - process.cwd(), - undefined, - promptForSettingsMock, - ); + await extensionManager.installOrUpdateExtension({ + source: sourceExtDir, + type: 'local', + }); - expect(promptForSettingsMock).toHaveBeenCalled(); + expect(mockPromptForSettings).toHaveBeenCalled(); }); it('should not prompt for settings if promptForSettings is false', async () => { @@ -1106,10 +1021,17 @@ This extension will run the following MCP servers: ], }); - await installOrUpdateExtension( - { source: sourceExtDir, type: 'local' }, - async (_) => true, - ); + extensionManager = new ExtensionManager({ + workspaceDir: tempWorkspaceDir, + requestConsent: mockRequestConsent, + requestSetting: null, + loadedSettings: loadSettings(tempWorkspaceDir), + }); + + await extensionManager.installOrUpdateExtension({ + source: sourceExtDir, + type: 'local', + }); }); it('should only prompt for new settings on update, and preserve old settings', async () => { @@ -1127,14 +1049,12 @@ This extension will run the following MCP servers: ], }); + mockPromptForSettings.mockResolvedValueOnce('old-api-key'); // Install it so it exists in the userExtensionsDir - await installOrUpdateExtension( - { source: oldSourceExtDir, type: 'local' }, - async (_) => true, - process.cwd(), - undefined, - async () => 'old-api-key', - ); + await extensionManager.installOrUpdateExtension({ + source: oldSourceExtDir, + type: 'local', + }); const envPath = new ExtensionStorage( 'my-local-extension', @@ -1142,6 +1062,7 @@ This extension will run the following MCP servers: expect(fs.existsSync(envPath)).toBe(true); let envContent = fs.readFileSync(envPath, 'utf-8'); expect(envContent).toContain('MY_API_KEY=old-api-key'); + expect(mockPromptForSettings).toHaveBeenCalledTimes(1); // 2. Create the "new" version of the extension in a new source directory. const newSourceExtDir = createExtension({ @@ -1162,27 +1083,19 @@ This extension will run the following MCP servers: ], }); - const previousExtensionConfig = loadExtensionConfig({ - extensionDir: path.join(userExtensionsDir, 'my-local-extension'), - workspaceDir: process.cwd(), - extensionEnablementManager: new ExtensionEnablementManager(), - }); - - const promptForSettingsMock = vi.fn( - async (_: ExtensionSetting): Promise => 'new-setting-value', + const previousExtensionConfig = extensionManager.loadExtensionConfig( + path.join(userExtensionsDir, 'my-local-extension'), ); + mockPromptForSettings.mockResolvedValueOnce('new-setting-value'); // 3. Call installOrUpdateExtension to perform the update. - await installOrUpdateExtension( + await extensionManager.installOrUpdateExtension( { source: newSourceExtDir, type: 'local' }, - async (_) => true, - process.cwd(), previousExtensionConfig, - promptForSettingsMock, ); - expect(promptForSettingsMock).toHaveBeenCalledTimes(1); - expect(promptForSettingsMock).toHaveBeenCalledWith( + expect(mockPromptForSettings).toHaveBeenCalledTimes(2); + expect(mockPromptForSettings).toHaveBeenCalledWith( expect.objectContaining({ name: 'New Setting' }), ); @@ -1206,10 +1119,11 @@ This extension will run the following MCP servers: }, ], }); - await installOrUpdateExtension( - { source: oldSourceExtDir, type: 'local', autoUpdate: true }, - async () => true, - ); + await extensionManager.installOrUpdateExtension({ + source: oldSourceExtDir, + type: 'local', + autoUpdate: true, + }); // 2. Create new version with different settings const newSourceExtDir = createExtension({ @@ -1225,18 +1139,14 @@ This extension will run the following MCP servers: ], }); - const previousExtensionConfig = loadExtensionConfig({ - extensionDir: path.join(userExtensionsDir, 'my-auto-update-ext'), - workspaceDir: process.cwd(), - extensionEnablementManager: new ExtensionEnablementManager(), - }); + const previousExtensionConfig = extensionManager.loadExtensionConfig( + path.join(userExtensionsDir, 'my-auto-update-ext'), + ); // 3. Attempt to update and assert it fails await expect( - installOrUpdateExtension( + extensionManager.installOrUpdateExtension( { source: newSourceExtDir, type: 'local', autoUpdate: true }, - async () => true, - process.cwd(), previousExtensionConfig, ), ).rejects.toThrow( @@ -1252,10 +1162,10 @@ This extension will run the following MCP servers: }); await expect( - installOrUpdateExtension( - { source: sourceExtDir, type: 'local' }, - async (_) => true, - ), + extensionManager.installOrUpdateExtension({ + source: sourceExtDir, + type: 'local', + }), ).rejects.toThrow('Invalid extension name: "bad_name"'); }); @@ -1300,10 +1210,10 @@ This extension will run the following MCP servers: join(tempDir, extensionName), ); - await installOrUpdateExtension( - { source: gitUrl, type: 'github-release' }, - async () => true, - ); + await extensionManager.installOrUpdateExtension({ + source: gitUrl, + type: 'github-release', + }); expect(fs.existsSync(targetExtDir)).toBe(true); const metadataPath = path.join(targetExtDir, INSTALL_METADATA_FILENAME); @@ -1323,17 +1233,15 @@ This extension will run the following MCP servers: errorMessage: 'download failed', type: 'github-release', }); - const requestConsent = vi.fn().mockResolvedValue(true); - await installOrUpdateExtension( + await extensionManager.installOrUpdateExtension( { source: gitUrl, type: 'github-release' }, // Use github-release to force consent - requestConsent, ); // It gets called once to ask for a git clone, and once to consent to // the actual extension features. - expect(requestConsent).toHaveBeenCalledTimes(2); - expect(requestConsent).toHaveBeenCalledWith( + expect(mockRequestConsent).toHaveBeenCalledTimes(2); + expect(mockRequestConsent).toHaveBeenCalledWith( expect.stringContaining( 'Would you like to attempt to install via "git clone" instead?', ), @@ -1354,18 +1262,18 @@ This extension will run the following MCP servers: errorMessage: 'download failed', type: 'github-release', }); - const requestConsent = vi.fn().mockResolvedValue(false); + mockRequestConsent.mockResolvedValue(false); await expect( - installOrUpdateExtension( - { source: gitUrl, type: 'github-release' }, - requestConsent, - ), + extensionManager.installOrUpdateExtension({ + source: gitUrl, + type: 'github-release', + }), ).rejects.toThrow( `Failed to install extension ${gitUrl}: download failed`, ); - expect(requestConsent).toHaveBeenCalledExactlyOnceWith( + expect(mockRequestConsent).toHaveBeenCalledExactlyOnceWith( expect.stringContaining( 'Would you like to attempt to install via "git clone" instead?', ), @@ -1379,16 +1287,15 @@ This extension will run the following MCP servers: failureReason: 'no release data', type: 'github-release', }); - const requestConsent = vi.fn().mockResolvedValue(true); - await installOrUpdateExtension( - { source: gitUrl, type: 'git' }, - requestConsent, - ); + await extensionManager.installOrUpdateExtension({ + source: gitUrl, + type: 'git', + }); // We should not see the request to use git clone, this is a repo that // has no github releases so it is the only install method. - expect(requestConsent).toHaveBeenCalledExactlyOnceWith( + expect(mockRequestConsent).toHaveBeenCalledExactlyOnceWith( expect.stringContaining( 'Installing extension "gemini-test-extension"', ), @@ -1410,14 +1317,12 @@ This extension will run the following MCP servers: errorMessage: 'No release data found', type: 'github-release', }); - const requestConsent = vi.fn().mockResolvedValue(true); - await installOrUpdateExtension( + await extensionManager.installOrUpdateExtension( { source: gitUrl, type: 'github-release' }, // Note the type - requestConsent, ); - expect(requestConsent).toHaveBeenCalledWith( + expect(mockRequestConsent).toHaveBeenCalledWith( expect.stringContaining( 'Would you like to attempt to install via "git clone" instead?', ), @@ -1435,7 +1340,7 @@ This extension will run the following MCP servers: version: '1.0.0', }); - await uninstallExtension('my-local-extension', false); + await extensionManager.uninstallExtension('my-local-extension', false); expect(fs.existsSync(sourceExtDir)).toBe(false); }); @@ -1452,16 +1357,16 @@ This extension will run the following MCP servers: version: '1.0.0', }); - await uninstallExtension('my-local-extension', false); + await extensionManager.uninstallExtension('my-local-extension', false); expect(fs.existsSync(sourceExtDir)).toBe(false); - expect(loadExtensions(new ExtensionEnablementManager())).toHaveLength(1); + expect(extensionManager.loadExtensions()).toHaveLength(1); expect(fs.existsSync(otherExtDir)).toBe(true); }); it('should throw an error if the extension does not exist', async () => { await expect( - uninstallExtension('nonexistent-extension', false), + extensionManager.uninstallExtension('nonexistent-extension', false), ).rejects.toThrow('Extension not found.'); }); @@ -1477,7 +1382,10 @@ This extension will run the following MCP servers: }, }); - await uninstallExtension('my-local-extension', isUpdate); + await extensionManager.uninstallExtension( + 'my-local-extension', + isUpdate, + ); if (isUpdate) { expect(mockLogExtensionUninstall).not.toHaveBeenCalled(); @@ -1501,7 +1409,7 @@ This extension will run the following MCP servers: const enablementManager = new ExtensionEnablementManager(); enablementManager.enable('test-extension', true, '/some/scope'); - await uninstallExtension('test-extension', isUpdate); + await extensionManager.uninstallExtension('test-extension', isUpdate); const config = enablementManager.readConfig()['test-extension']; if (isUpdate) { @@ -1525,7 +1433,7 @@ This extension will run the following MCP servers: }, }); - await uninstallExtension(gitUrl, false); + await extensionManager.uninstallExtension(gitUrl, false); expect(fs.existsSync(sourceExtDir)).toBe(false); expect(mockLogExtensionUninstall).toHaveBeenCalled(); @@ -1545,7 +1453,7 @@ This extension will run the following MCP servers: }); await expect( - uninstallExtension( + extensionManager.uninstallExtension( 'https://github.com/google/no-metadata-extension', false, ), @@ -1561,11 +1469,7 @@ This extension will run the following MCP servers: version: '1.0.0', }); - disableExtension( - 'my-extension', - SettingScope.User, - new ExtensionEnablementManager(), - ); + extensionManager.disableExtension('my-extension', SettingScope.User); expect( isEnabled({ name: 'my-extension', @@ -1581,12 +1485,7 @@ This extension will run the following MCP servers: version: '1.0.0', }); - disableExtension( - 'my-extension', - SettingScope.Workspace, - new ExtensionEnablementManager(), - tempWorkspaceDir, - ); + extensionManager.disableExtension('my-extension', SettingScope.Workspace); expect( isEnabled({ name: 'my-extension', @@ -1608,16 +1507,8 @@ This extension will run the following MCP servers: version: '1.0.0', }); - disableExtension( - 'my-extension', - SettingScope.User, - new ExtensionEnablementManager(), - ); - disableExtension( - 'my-extension', - SettingScope.User, - new ExtensionEnablementManager(), - ); + extensionManager.disableExtension('my-extension', SettingScope.User); + extensionManager.disableExtension('my-extension', SettingScope.User); expect( isEnabled({ name: 'my-extension', @@ -1628,11 +1519,7 @@ This extension will run the following MCP servers: it('should throw an error if you request system scope', () => { expect(() => - disableExtension( - 'my-extension', - SettingScope.System, - new ExtensionEnablementManager(), - ), + extensionManager.disableExtension('my-extension', SettingScope.System), ).toThrow('System and SystemDefaults scopes are not supported.'); }); @@ -1647,11 +1534,7 @@ This extension will run the following MCP servers: }, }); - disableExtension( - 'ext1', - SettingScope.Workspace, - new ExtensionEnablementManager(), - ); + extensionManager.disableExtension('ext1', SettingScope.Workspace); expect(mockLogExtensionDisable).toHaveBeenCalled(); expect(ExtensionDisableEvent).toHaveBeenCalledWith( @@ -1668,8 +1551,7 @@ This extension will run the following MCP servers: }); const getActiveExtensions = (): GeminiCLIExtension[] => { - const manager = new ExtensionEnablementManager(); - const extensions = loadExtensions(manager); + const extensions = extensionManager.loadExtensions(); return extensions.filter((e) => e.isActive); }; @@ -1679,12 +1561,11 @@ This extension will run the following MCP servers: name: 'ext1', version: '1.0.0', }); - const extensionEnablementManager = new ExtensionEnablementManager(); - disableExtension('ext1', SettingScope.User, extensionEnablementManager); + extensionManager.disableExtension('ext1', SettingScope.User); let activeExtensions = getActiveExtensions(); expect(activeExtensions).toHaveLength(0); - enableExtension('ext1', SettingScope.User, extensionEnablementManager); + extensionManager.enableExtension('ext1', SettingScope.User); activeExtensions = getActiveExtensions(); expect(activeExtensions).toHaveLength(1); expect(activeExtensions[0].name).toBe('ext1'); @@ -1696,20 +1577,11 @@ This extension will run the following MCP servers: name: 'ext1', version: '1.0.0', }); - const extensionEnablementManager = new ExtensionEnablementManager(); - disableExtension( - 'ext1', - SettingScope.Workspace, - extensionEnablementManager, - ); + extensionManager.disableExtension('ext1', SettingScope.Workspace); let activeExtensions = getActiveExtensions(); expect(activeExtensions).toHaveLength(0); - enableExtension( - 'ext1', - SettingScope.Workspace, - extensionEnablementManager, - ); + extensionManager.enableExtension('ext1', SettingScope.Workspace); activeExtensions = getActiveExtensions(); expect(activeExtensions).toHaveLength(1); expect(activeExtensions[0].name).toBe('ext1'); @@ -1725,17 +1597,8 @@ This extension will run the following MCP servers: type: 'local', }, }); - const extensionEnablementManager = new ExtensionEnablementManager(); - disableExtension( - 'ext1', - SettingScope.Workspace, - extensionEnablementManager, - ); - enableExtension( - 'ext1', - SettingScope.Workspace, - extensionEnablementManager, - ); + extensionManager.disableExtension('ext1', SettingScope.Workspace); + extensionManager.enableExtension('ext1', SettingScope.Workspace); expect(mockLogExtensionEnable).toHaveBeenCalled(); expect(ExtensionEnableEvent).toHaveBeenCalledWith( diff --git a/packages/cli/src/config/extension.ts b/packages/cli/src/config/extension.ts index 482f7eabbb..bafaba59a8 100644 --- a/packages/cli/src/config/extension.ts +++ b/packages/cli/src/config/extension.ts @@ -6,61 +6,12 @@ import type { MCPServerConfig, - GeminiCLIExtension, ExtensionInstallMetadata, } from '@google/gemini-cli-core'; -import { - GEMINI_DIR, - Storage, - Config, - ExtensionInstallEvent, - ExtensionUninstallEvent, - ExtensionUpdateEvent, - ExtensionDisableEvent, - ExtensionEnableEvent, - logExtensionEnable, - logExtensionInstallEvent, - logExtensionUninstall, - logExtensionUpdateEvent, - logExtensionDisable, - debugLogger, -} from '@google/gemini-cli-core'; import * as fs from 'node:fs'; import * as path from 'node:path'; -import * as os from 'node:os'; -import { SettingScope, loadSettings } from '../config/settings.js'; -import { getErrorMessage } from '../utils/errors.js'; -import { - recursivelyHydrateStrings, - type JsonObject, -} from './extensions/variables.js'; -import { isWorkspaceTrusted } from './trustedFolders.js'; -import { randomUUID, createHash } from 'node:crypto'; -import { - cloneFromGit, - downloadFromGitHubRelease, - tryParseGithubUrl, -} from './extensions/github.js'; -import type { LoadExtensionContext } from './extensions/variableSchema.js'; -import { ExtensionEnablementManager } from './extensions/extensionEnablement.js'; -import chalk from 'chalk'; -import { resolveEnvVarsInObject } from '../utils/envVarResolver.js'; -import type { ConfirmationRequest } from '../ui/types.js'; -import { escapeAnsiCtrlCodes } from '../ui/utils/textUtils.js'; -import { - getEnvContents, - maybePromptForSettings, - type ExtensionSetting, -} from './extensions/extensionSettings.js'; - -export const EXTENSIONS_DIRECTORY_NAME = path.join(GEMINI_DIR, 'extensions'); - -export const EXTENSIONS_CONFIG_FILENAME = 'gemini-extension.json'; -export const INSTALL_METADATA_FILENAME = '.gemini-extension-install.json'; -export const EXTENSION_SETTINGS_FILENAME = '.env'; - -export const INSTALL_WARNING_MESSAGE = - '**The extension you are about to install may have been created by a third-party developer and sourced from a public repository. Google does not vet, endorse, or guarantee the functionality or security of extensions. Please carefully inspect any extension and its source code before installing to understand the permissions it requires and the actions it may perform.**'; +import { INSTALL_METADATA_FILENAME } from './extensions/variables.js'; +import type { ExtensionSetting } from './extensions/extensionSettings.js'; /** * Extension definition as written to disk in gemini-extension.json files. @@ -84,190 +35,6 @@ export interface ExtensionUpdateInfo { updatedVersion: string; } -export class ExtensionStorage { - private readonly extensionName: string; - - constructor(extensionName: string) { - this.extensionName = extensionName; - } - - getExtensionDir(): string { - return path.join( - ExtensionStorage.getUserExtensionsDir(), - this.extensionName, - ); - } - - getConfigPath(): string { - return path.join(this.getExtensionDir(), EXTENSIONS_CONFIG_FILENAME); - } - - getEnvFilePath(): string { - return path.join(this.getExtensionDir(), EXTENSION_SETTINGS_FILENAME); - } - - static getUserExtensionsDir(): string { - const storage = new Storage(os.homedir()); - return storage.getExtensionsDir(); - } - - static async createTmpDir(): Promise { - return await fs.promises.mkdtemp( - path.join(os.tmpdir(), 'gemini-extension'), - ); - } -} - -export async function copyExtension( - source: string, - destination: string, -): Promise { - await fs.promises.cp(source, destination, { recursive: true }); -} - -function getTelemetryConfig(cwd: string) { - const settings = loadSettings(cwd); - const config = new Config({ - telemetry: settings.merged.telemetry, - interactive: false, - sessionId: randomUUID(), - targetDir: cwd, - cwd, - model: '', - debugMode: false, - }); - return config; -} - -export function loadExtensions( - extensionEnablementManager: ExtensionEnablementManager, - workspaceDir: string = process.cwd(), -): GeminiCLIExtension[] { - const extensionsDir = ExtensionStorage.getUserExtensionsDir(); - if (!fs.existsSync(extensionsDir)) { - return []; - } - - const extensions: GeminiCLIExtension[] = []; - for (const subdir of fs.readdirSync(extensionsDir)) { - const extensionDir = path.join(extensionsDir, subdir); - - const extension = loadExtension({ - extensionDir, - workspaceDir, - extensionEnablementManager, - }); - if (extension != null) { - extensions.push(extension); - } - } - - const uniqueExtensions = new Map(); - - for (const extension of extensions) { - if (!uniqueExtensions.has(extension.name)) { - uniqueExtensions.set(extension.name, extension); - } - } - - return Array.from(uniqueExtensions.values()); -} - -export function loadExtension( - context: LoadExtensionContext, -): GeminiCLIExtension | null { - const { extensionDir, workspaceDir, extensionEnablementManager } = context; - if (!fs.statSync(extensionDir).isDirectory()) { - return null; - } - - const installMetadata = loadInstallMetadata(extensionDir); - let effectiveExtensionPath = extensionDir; - - if (installMetadata?.type === 'link') { - effectiveExtensionPath = installMetadata.source; - } - - try { - let config = loadExtensionConfig({ - extensionDir: effectiveExtensionPath, - workspaceDir, - extensionEnablementManager, - }); - - const customEnv = getEnvContents(new ExtensionStorage(config.name)); - config = resolveEnvVarsInObject(config, customEnv); - - if (config.mcpServers) { - config.mcpServers = Object.fromEntries( - Object.entries(config.mcpServers).map(([key, value]) => [ - key, - filterMcpConfig(value), - ]), - ); - } - - const contextFiles = getContextFileNames(config) - .map((contextFileName) => - path.join(effectiveExtensionPath, contextFileName), - ) - .filter((contextFilePath) => fs.existsSync(contextFilePath)); - - return { - name: config.name, - version: config.version, - path: effectiveExtensionPath, - contextFiles, - installMetadata, - mcpServers: config.mcpServers, - excludeTools: config.excludeTools, - isActive: extensionEnablementManager.isEnabled(config.name, workspaceDir), - id: getExtensionId(config, installMetadata), - }; - } catch (e) { - debugLogger.error( - `Warning: Skipping extension in ${effectiveExtensionPath}: ${getErrorMessage( - e, - )}`, - ); - return null; - } -} - -export function loadExtensionByName( - name: string, - extensionEnablementManager: ExtensionEnablementManager, - workspaceDir: string = process.cwd(), -): GeminiCLIExtension | null { - const userExtensionsDir = ExtensionStorage.getUserExtensionsDir(); - if (!fs.existsSync(userExtensionsDir)) { - return null; - } - - for (const subdir of fs.readdirSync(userExtensionsDir)) { - const extensionDir = path.join(userExtensionsDir, subdir); - if (!fs.statSync(extensionDir).isDirectory()) { - continue; - } - const extension = loadExtension({ - extensionDir, - workspaceDir, - extensionEnablementManager, - }); - if (extension && extension.name.toLowerCase() === name.toLowerCase()) { - return extension; - } - } - - return null; -} - -function filterMcpConfig(original: MCPServerConfig): MCPServerConfig { - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const { trust, ...rest } = original; - return Object.freeze(rest); -} - export function loadInstallMetadata( extensionDir: string, ): ExtensionInstallMetadata | undefined { @@ -280,612 +47,3 @@ export function loadInstallMetadata( return undefined; } } - -function getContextFileNames(config: ExtensionConfig): string[] { - if (!config.contextFileName) { - return ['GEMINI.md']; - } else if (!Array.isArray(config.contextFileName)) { - return [config.contextFileName]; - } - return config.contextFileName; -} - -/** - * Requests consent from the user to perform an action, by reading a Y/n - * character from stdin. - * - * This should not be called from interactive mode as it will break the CLI. - * - * @param consentDescription The description of the thing they will be consenting to. - * @returns boolean, whether they consented or not. - */ -export async function requestConsentNonInteractive( - consentDescription: string, -): Promise { - debugLogger.log(consentDescription); - const result = await promptForConsentNonInteractive( - 'Do you want to continue? [Y/n]: ', - ); - return result; -} - -/** - * Requests consent from the user to perform an action, in interactive mode. - * - * This should not be called from non-interactive mode as it will not work. - * - * @param consentDescription The description of the thing they will be consenting to. - * @param setExtensionUpdateConfirmationRequest A function to actually add a prompt to the UI. - * @returns boolean, whether they consented or not. - */ -export async function requestConsentInteractive( - consentDescription: string, - addExtensionUpdateConfirmationRequest: (value: ConfirmationRequest) => void, -): Promise { - return await promptForConsentInteractive( - consentDescription + '\n\nDo you want to continue?', - addExtensionUpdateConfirmationRequest, - ); -} - -/** - * Asks users a prompt and awaits for a y/n response on stdin. - * - * This should not be called from interactive mode as it will break the CLI. - * - * @param prompt A yes/no prompt to ask the user - * @returns Whether or not the user answers 'y' (yes). Defaults to 'yes' on enter. - */ -async function promptForConsentNonInteractive( - prompt: string, -): Promise { - const readline = await import('node:readline'); - const rl = readline.createInterface({ - input: process.stdin, - output: process.stdout, - }); - - return new Promise((resolve) => { - rl.question(prompt, (answer) => { - rl.close(); - resolve(['y', ''].includes(answer.trim().toLowerCase())); - }); - }); -} - -/** - * Asks users an interactive yes/no prompt. - * - * This should not be called from non-interactive mode as it will break the CLI. - * - * @param prompt A markdown prompt to ask the user - * @param setExtensionUpdateConfirmationRequest Function to update the UI state with the confirmation request. - * @returns Whether or not the user answers yes. - */ -async function promptForConsentInteractive( - prompt: string, - addExtensionUpdateConfirmationRequest: (value: ConfirmationRequest) => void, -): Promise { - return await new Promise((resolve) => { - addExtensionUpdateConfirmationRequest({ - prompt, - onConfirm: (resolvedConfirmed) => { - resolve(resolvedConfirmed); - }, - }); - }); -} - -export function hashValue(value: string): string { - return createHash('sha256').update(value).digest('hex'); -} - -export async function installOrUpdateExtension( - installMetadata: ExtensionInstallMetadata, - requestConsent: (consent: string) => Promise, - cwd: string = process.cwd(), - previousExtensionConfig?: ExtensionConfig, - requestSetting?: (setting: ExtensionSetting) => Promise, -): Promise { - const isUpdate = !!previousExtensionConfig; - const telemetryConfig = getTelemetryConfig(cwd); - let newExtensionConfig: ExtensionConfig | null = null; - let localSourcePath: string | undefined; - const extensionEnablementManager = new ExtensionEnablementManager(); - // path.join(tempDir, EXTENSION_SETTINGS_FILENAME) - try { - const settings = loadSettings(cwd).merged; - if (!isWorkspaceTrusted(settings).isTrusted) { - throw new Error( - `Could not install extension from untrusted folder at ${installMetadata.source}`, - ); - } - - const extensionsDir = ExtensionStorage.getUserExtensionsDir(); - await fs.promises.mkdir(extensionsDir, { recursive: true }); - - if ( - !path.isAbsolute(installMetadata.source) && - (installMetadata.type === 'local' || installMetadata.type === 'link') - ) { - installMetadata.source = path.resolve(cwd, installMetadata.source); - } - - let tempDir: string | undefined; - - if ( - installMetadata.type === 'git' || - installMetadata.type === 'github-release' - ) { - tempDir = await ExtensionStorage.createTmpDir(); - const parsedGithubParts = tryParseGithubUrl(installMetadata.source); - if (!parsedGithubParts) { - await cloneFromGit(installMetadata, tempDir); - installMetadata.type = 'git'; - } else { - const result = await downloadFromGitHubRelease( - installMetadata, - tempDir, - parsedGithubParts, - ); - if (result.success) { - installMetadata.type = result.type; - installMetadata.releaseTag = result.tagName; - } else if ( - // This repo has no github releases, and wasn't explicitly installed - // from a github release, unconditionally just clone it. - (result.failureReason === 'no release data' && - installMetadata.type === 'git') || - // Otherwise ask the user if they would like to try a git clone. - (await requestConsent( - `Error downloading github release for ${installMetadata.source} with the following error: ${result.errorMessage}.\n\nWould you like to attempt to install via "git clone" instead?`, - )) - ) { - await cloneFromGit(installMetadata, tempDir); - installMetadata.type = 'git'; - } else { - throw new Error( - `Failed to install extension ${installMetadata.source}: ${result.errorMessage}`, - ); - } - } - localSourcePath = tempDir; - } else if ( - installMetadata.type === 'local' || - installMetadata.type === 'link' - ) { - localSourcePath = installMetadata.source; - } else { - throw new Error(`Unsupported install type: ${installMetadata.type}`); - } - - try { - newExtensionConfig = loadExtensionConfig({ - extensionDir: localSourcePath, - workspaceDir: cwd, - extensionEnablementManager, - }); - - if (isUpdate && previousExtensionConfig && installMetadata.autoUpdate) { - const oldSettings = new Set( - previousExtensionConfig.settings?.map((s) => s.name) || [], - ); - const newSettings = new Set( - newExtensionConfig.settings?.map((s) => s.name) || [], - ); - - const settingsAreEqual = - oldSettings.size === newSettings.size && - [...oldSettings].every((value) => newSettings.has(value)); - - if (!settingsAreEqual) { - throw new Error( - `Extension "${newExtensionConfig.name}" has settings changes and cannot be auto-updated. Please update manually.`, - ); - } - } - - const newExtensionName = newExtensionConfig.name; - if (!isUpdate) { - const installedExtensions = loadExtensions( - new ExtensionEnablementManager(), - cwd, - ); - if ( - installedExtensions.some( - (installed) => installed.name === newExtensionName, - ) - ) { - throw new Error( - `Extension "${newExtensionName}" is already installed. Please uninstall it first.`, - ); - } - } - - await maybeRequestConsentOrFail( - newExtensionConfig, - requestConsent, - previousExtensionConfig, - ); - - const extensionStorage = new ExtensionStorage(newExtensionName); - const destinationPath = extensionStorage.getExtensionDir(); - let previousSettings: Record | undefined; - if (isUpdate) { - previousSettings = getEnvContents(extensionStorage); - await uninstallExtension(newExtensionName, isUpdate, cwd); - } - - await fs.promises.mkdir(destinationPath, { recursive: true }); - if (requestSetting !== undefined) { - if (isUpdate && previousExtensionConfig) { - await maybePromptForSettings( - newExtensionConfig, - requestSetting, - previousExtensionConfig, - previousSettings, - ); - } else if (!isUpdate) { - await maybePromptForSettings(newExtensionConfig, requestSetting); - } - } - - if ( - installMetadata.type === 'local' || - installMetadata.type === 'git' || - installMetadata.type === 'github-release' - ) { - await copyExtension(localSourcePath, destinationPath); - } - - const metadataString = JSON.stringify(installMetadata, null, 2); - const metadataPath = path.join( - destinationPath, - INSTALL_METADATA_FILENAME, - ); - await fs.promises.writeFile(metadataPath, metadataString); - } finally { - if (tempDir) { - await fs.promises.rm(tempDir, { recursive: true, force: true }); - } - } - if (isUpdate) { - logExtensionUpdateEvent( - telemetryConfig, - new ExtensionUpdateEvent( - hashValue(newExtensionConfig.name), - getExtensionId(newExtensionConfig, installMetadata), - newExtensionConfig.version, - previousExtensionConfig.version, - installMetadata.type, - 'success', - ), - ); - } else { - logExtensionInstallEvent( - telemetryConfig, - new ExtensionInstallEvent( - hashValue(newExtensionConfig.name), - getExtensionId(newExtensionConfig, installMetadata), - newExtensionConfig.version, - installMetadata.type, - 'success', - ), - ); - enableExtension( - newExtensionConfig.name, - SettingScope.User, - extensionEnablementManager, - ); - } - - return newExtensionConfig!.name; - } catch (error) { - // Attempt to load config from the source path even if installation fails - // to get the name and version for logging. - if (!newExtensionConfig && localSourcePath) { - try { - newExtensionConfig = loadExtensionConfig({ - extensionDir: localSourcePath, - workspaceDir: cwd, - extensionEnablementManager, - }); - } catch { - // Ignore error, this is just for logging. - } - } - const config = newExtensionConfig ?? previousExtensionConfig; - const extensionId = config - ? getExtensionId(config, installMetadata) - : undefined; - if (isUpdate) { - logExtensionUpdateEvent( - telemetryConfig, - new ExtensionUpdateEvent( - hashValue(config?.name ?? ''), - extensionId ?? '', - newExtensionConfig?.version ?? '', - previousExtensionConfig.version, - installMetadata.type, - 'error', - ), - ); - } else { - logExtensionInstallEvent( - telemetryConfig, - new ExtensionInstallEvent( - hashValue(newExtensionConfig?.name ?? ''), - extensionId ?? '', - newExtensionConfig?.version ?? '', - installMetadata.type, - 'error', - ), - ); - } - throw error; - } -} - -/** - * Builds a consent string for installing an extension based on it's - * extensionConfig. - */ -function extensionConsentString(extensionConfig: ExtensionConfig): string { - const sanitizedConfig = escapeAnsiCtrlCodes(extensionConfig); - const output: string[] = []; - const mcpServerEntries = Object.entries(sanitizedConfig.mcpServers || {}); - output.push(`Installing extension "${sanitizedConfig.name}".`); - output.push(INSTALL_WARNING_MESSAGE); - - if (mcpServerEntries.length) { - output.push('This extension will run the following MCP servers:'); - for (const [key, mcpServer] of mcpServerEntries) { - const isLocal = !!mcpServer.command; - const source = - mcpServer.httpUrl ?? - `${mcpServer.command || ''}${mcpServer.args ? ' ' + mcpServer.args.join(' ') : ''}`; - output.push(` * ${key} (${isLocal ? 'local' : 'remote'}): ${source}`); - } - } - if (sanitizedConfig.contextFileName) { - output.push( - `This extension will append info to your gemini.md context using ${sanitizedConfig.contextFileName}`, - ); - } - if (sanitizedConfig.excludeTools) { - output.push( - `This extension will exclude the following core tools: ${sanitizedConfig.excludeTools}`, - ); - } - return output.join('\n'); -} - -/** - * Requests consent from the user to install an extension (extensionConfig), if - * there is any difference between the consent string for `extensionConfig` and - * `previousExtensionConfig`. - * - * Always requests consent if previousExtensionConfig is null. - * - * Throws if the user does not consent. - */ -async function maybeRequestConsentOrFail( - extensionConfig: ExtensionConfig, - requestConsent: (consent: string) => Promise, - previousExtensionConfig?: ExtensionConfig, -) { - const extensionConsent = extensionConsentString(extensionConfig); - if (previousExtensionConfig) { - const previousExtensionConsent = extensionConsentString( - previousExtensionConfig, - ); - if (previousExtensionConsent === extensionConsent) { - return; - } - } - if (!(await requestConsent(extensionConsent))) { - throw new Error(`Installation cancelled for "${extensionConfig.name}".`); - } -} - -export function validateName(name: string) { - if (!/^[a-zA-Z0-9-]+$/.test(name)) { - throw new Error( - `Invalid extension name: "${name}". Only letters (a-z, A-Z), numbers (0-9), and dashes (-) are allowed.`, - ); - } -} - -export function loadExtensionConfig( - context: LoadExtensionContext, -): ExtensionConfig { - const { extensionDir, workspaceDir } = context; - const configFilePath = path.join(extensionDir, EXTENSIONS_CONFIG_FILENAME); - if (!fs.existsSync(configFilePath)) { - throw new Error(`Configuration file not found at ${configFilePath}`); - } - try { - const configContent = fs.readFileSync(configFilePath, 'utf-8'); - const rawConfig = JSON.parse(configContent) as ExtensionConfig; - if (!rawConfig.name || !rawConfig.version) { - throw new Error( - `Invalid configuration in ${configFilePath}: missing ${!rawConfig.name ? '"name"' : '"version"'}`, - ); - } - const installDir = new ExtensionStorage(rawConfig.name).getExtensionDir(); - const config = recursivelyHydrateStrings( - rawConfig as unknown as JsonObject, - { - extensionPath: installDir, - workspacePath: workspaceDir, - '/': path.sep, - pathSeparator: path.sep, - }, - ) as unknown as ExtensionConfig; - - validateName(config.name); - return config; - } catch (e) { - throw new Error( - `Failed to load extension config from ${configFilePath}: ${getErrorMessage( - e, - )}`, - ); - } -} - -export async function uninstallExtension( - extensionIdentifier: string, - isUpdate: boolean, - cwd: string = process.cwd(), -): Promise { - const installedExtensions = loadExtensions( - new ExtensionEnablementManager(), - cwd, - ); - const extension = installedExtensions.find( - (installed) => - installed.name.toLowerCase() === extensionIdentifier.toLowerCase() || - installed.installMetadata?.source.toLowerCase() === - extensionIdentifier.toLowerCase(), - ); - if (!extension) { - throw new Error(`Extension not found.`); - } - const storage = new ExtensionStorage(extension.name); - - await fs.promises.rm(storage.getExtensionDir(), { - recursive: true, - force: true, - }); - - // The rest of the cleanup below here is only for true uninstalls, not - // uninstalls related to updates. - if (isUpdate) return; - - const manager = new ExtensionEnablementManager([extension.name]); - manager.remove(extension.name); - - const telemetryConfig = getTelemetryConfig(cwd); - logExtensionUninstall( - telemetryConfig, - new ExtensionUninstallEvent( - hashValue(extension.name), - extension.id, - 'success', - ), - ); -} - -export function toOutputString( - extension: GeminiCLIExtension, - workspaceDir: string, -): string { - const manager = new ExtensionEnablementManager(); - const userEnabled = manager.isEnabled(extension.name, os.homedir()); - const workspaceEnabled = manager.isEnabled(extension.name, workspaceDir); - - const status = workspaceEnabled ? chalk.green('✓') : chalk.red('✗'); - let output = `${status} ${extension.name} (${extension.version})`; - output += `\n ID: ${extension.id}`; - output += `\n Path: ${extension.path}`; - if (extension.installMetadata) { - output += `\n Source: ${extension.installMetadata.source} (Type: ${extension.installMetadata.type})`; - if (extension.installMetadata.ref) { - output += `\n Ref: ${extension.installMetadata.ref}`; - } - if (extension.installMetadata.releaseTag) { - output += `\n Release tag: ${extension.installMetadata.releaseTag}`; - } - } - output += `\n Enabled (User): ${userEnabled}`; - output += `\n Enabled (Workspace): ${workspaceEnabled}`; - if (extension.contextFiles.length > 0) { - output += `\n Context files:`; - extension.contextFiles.forEach((contextFile) => { - output += `\n ${contextFile}`; - }); - } - if (extension.mcpServers) { - output += `\n MCP servers:`; - Object.keys(extension.mcpServers).forEach((key) => { - output += `\n ${key}`; - }); - } - if (extension.excludeTools) { - output += `\n Excluded tools:`; - extension.excludeTools.forEach((tool) => { - output += `\n ${tool}`; - }); - } - return output; -} - -export function disableExtension( - name: string, - scope: SettingScope, - extensionEnablementManager: ExtensionEnablementManager, - cwd: string = process.cwd(), -) { - const config = getTelemetryConfig(cwd); - if (scope === SettingScope.System || scope === SettingScope.SystemDefaults) { - throw new Error('System and SystemDefaults scopes are not supported.'); - } - const extension = loadExtensionByName(name, extensionEnablementManager, cwd); - if (!extension) { - throw new Error(`Extension with name ${name} does not exist.`); - } - - const scopePath = scope === SettingScope.Workspace ? cwd : os.homedir(); - extensionEnablementManager.disable(name, true, scopePath); - logExtensionDisable( - config, - new ExtensionDisableEvent(hashValue(name), extension.id, scope), - ); -} - -export function enableExtension( - name: string, - scope: SettingScope, - extensionEnablementManager: ExtensionEnablementManager, - cwd: string = process.cwd(), -) { - if (scope === SettingScope.System || scope === SettingScope.SystemDefaults) { - throw new Error('System and SystemDefaults scopes are not supported.'); - } - const extension = loadExtensionByName(name, extensionEnablementManager, cwd); - if (!extension) { - throw new Error(`Extension with name ${name} does not exist.`); - } - const scopePath = scope === SettingScope.Workspace ? cwd : os.homedir(); - extensionEnablementManager.enable(name, true, scopePath); - const config = getTelemetryConfig(cwd); - logExtensionEnable( - config, - new ExtensionEnableEvent(hashValue(name), extension.id, scope), - ); -} - -function getExtensionId( - config: ExtensionConfig, - installMetadata?: ExtensionInstallMetadata, -): string { - // IDs are created by hashing details of the installation source in order to - // deduplicate extensions with conflicting names and also obfuscate any - // potentially sensitive information such as private git urls, system paths, - // or project names. - let idValue = config.name; - const githubUrlParts = - installMetadata && - (installMetadata.type === 'git' || - installMetadata.type === 'github-release') - ? tryParseGithubUrl(installMetadata.source) - : null; - if (githubUrlParts) { - // For github repos, we use the https URI to the repo as the ID. - idValue = `https://github.com/${githubUrlParts.owner}/${githubUrlParts.repo}`; - } else { - idValue = installMetadata?.source ?? config.name; - } - return hashValue(idValue); -} diff --git a/packages/cli/src/config/extensions/consent.ts b/packages/cli/src/config/extensions/consent.ts new file mode 100644 index 0000000000..b6db1f647b --- /dev/null +++ b/packages/cli/src/config/extensions/consent.ts @@ -0,0 +1,162 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { debugLogger } from '@google/gemini-cli-core'; + +import type { ConfirmationRequest } from '../../ui/types.js'; +import { escapeAnsiCtrlCodes } from '../../ui/utils/textUtils.js'; +import type { ExtensionConfig } from '../extension.js'; + +export const INSTALL_WARNING_MESSAGE = + '**The extension you are about to install may have been created by a third-party developer and sourced from a public repository. Google does not vet, endorse, or guarantee the functionality or security of extensions. Please carefully inspect any extension and its source code before installing to understand the permissions it requires and the actions it may perform.**'; + +/** + * Requests consent from the user to perform an action, by reading a Y/n + * character from stdin. + * + * This should not be called from interactive mode as it will break the CLI. + * + * @param consentDescription The description of the thing they will be consenting to. + * @returns boolean, whether they consented or not. + */ +export async function requestConsentNonInteractive( + consentDescription: string, +): Promise { + debugLogger.log(consentDescription); + const result = await promptForConsentNonInteractive( + 'Do you want to continue? [Y/n]: ', + ); + return result; +} + +/** + * Requests consent from the user to perform an action, in interactive mode. + * + * This should not be called from non-interactive mode as it will not work. + * + * @param consentDescription The description of the thing they will be consenting to. + * @param setExtensionUpdateConfirmationRequest A function to actually add a prompt to the UI. + * @returns boolean, whether they consented or not. + */ +export async function requestConsentInteractive( + consentDescription: string, + addExtensionUpdateConfirmationRequest: (value: ConfirmationRequest) => void, +): Promise { + return await promptForConsentInteractive( + consentDescription + '\n\nDo you want to continue?', + addExtensionUpdateConfirmationRequest, + ); +} + +/** + * Asks users a prompt and awaits for a y/n response on stdin. + * + * This should not be called from interactive mode as it will break the CLI. + * + * @param prompt A yes/no prompt to ask the user + * @returns Whether or not the user answers 'y' (yes). Defaults to 'yes' on enter. + */ +async function promptForConsentNonInteractive( + prompt: string, +): Promise { + const readline = await import('node:readline'); + const rl = readline.createInterface({ + input: process.stdin, + output: process.stdout, + }); + + return new Promise((resolve) => { + rl.question(prompt, (answer) => { + rl.close(); + resolve(['y', ''].includes(answer.trim().toLowerCase())); + }); + }); +} + +/** + * Asks users an interactive yes/no prompt. + * + * This should not be called from non-interactive mode as it will break the CLI. + * + * @param prompt A markdown prompt to ask the user + * @param setExtensionUpdateConfirmationRequest Function to update the UI state with the confirmation request. + * @returns Whether or not the user answers yes. + */ +async function promptForConsentInteractive( + prompt: string, + addExtensionUpdateConfirmationRequest: (value: ConfirmationRequest) => void, +): Promise { + return await new Promise((resolve) => { + addExtensionUpdateConfirmationRequest({ + prompt, + onConfirm: (resolvedConfirmed) => { + resolve(resolvedConfirmed); + }, + }); + }); +} + +/** + * Builds a consent string for installing an extension based on it's + * extensionConfig. + */ +function extensionConsentString(extensionConfig: ExtensionConfig): string { + const sanitizedConfig = escapeAnsiCtrlCodes(extensionConfig); + const output: string[] = []; + const mcpServerEntries = Object.entries(sanitizedConfig.mcpServers || {}); + output.push(`Installing extension "${sanitizedConfig.name}".`); + output.push(INSTALL_WARNING_MESSAGE); + + if (mcpServerEntries.length) { + output.push('This extension will run the following MCP servers:'); + for (const [key, mcpServer] of mcpServerEntries) { + const isLocal = !!mcpServer.command; + const source = + mcpServer.httpUrl ?? + `${mcpServer.command || ''}${mcpServer.args ? ' ' + mcpServer.args.join(' ') : ''}`; + output.push(` * ${key} (${isLocal ? 'local' : 'remote'}): ${source}`); + } + } + if (sanitizedConfig.contextFileName) { + output.push( + `This extension will append info to your gemini.md context using ${sanitizedConfig.contextFileName}`, + ); + } + if (sanitizedConfig.excludeTools) { + output.push( + `This extension will exclude the following core tools: ${sanitizedConfig.excludeTools}`, + ); + } + return output.join('\n'); +} + +/** + * Requests consent from the user to install an extension (extensionConfig), if + * there is any difference between the consent string for `extensionConfig` and + * `previousExtensionConfig`. + * + * Always requests consent if previousExtensionConfig is null. + * + * Throws if the user does not consent. + */ +export async function maybeRequestConsentOrFail( + extensionConfig: ExtensionConfig, + requestConsent: (consent: string) => Promise, + previousExtensionConfig?: ExtensionConfig, +) { + const extensionConsent = extensionConsentString(extensionConfig); + if (previousExtensionConfig) { + const previousExtensionConsent = extensionConsentString( + previousExtensionConfig, + ); + if (previousExtensionConsent === extensionConsent) { + return; + } + } + if (!(await requestConsent(extensionConsent))) { + throw new Error(`Installation cancelled for "${extensionConfig.name}".`); + } +} diff --git a/packages/cli/src/config/extensions/extensionEnablement.ts b/packages/cli/src/config/extensions/extensionEnablement.ts index f5018df6a8..9994a4ecff 100644 --- a/packages/cli/src/config/extensions/extensionEnablement.ts +++ b/packages/cli/src/config/extensions/extensionEnablement.ts @@ -7,7 +7,7 @@ import fs from 'node:fs'; import path from 'node:path'; import type { GeminiCLIExtension } from '@google/gemini-cli-core'; -import { ExtensionStorage } from '../extension.js'; +import { ExtensionStorage } from './storage.js'; export interface ExtensionEnablementConfig { overrides: string[]; diff --git a/packages/cli/src/config/extensions/extensionSettings.test.ts b/packages/cli/src/config/extensions/extensionSettings.test.ts index e05c573f3b..9beb8a4284 100644 --- a/packages/cli/src/config/extensions/extensionSettings.test.ts +++ b/packages/cli/src/config/extensions/extensionSettings.test.ts @@ -13,7 +13,7 @@ import { type ExtensionSetting, } from './extensionSettings.js'; import type { ExtensionConfig } from '../extension.js'; -import { ExtensionStorage } from '../extension.js'; +import { ExtensionStorage } from './storage.js'; import prompts from 'prompts'; import * as fsPromises from 'node:fs/promises'; import * as fs from 'node:fs'; diff --git a/packages/cli/src/config/extensions/extensionSettings.ts b/packages/cli/src/config/extensions/extensionSettings.ts index dbc28f8e07..55eb70b83a 100644 --- a/packages/cli/src/config/extensions/extensionSettings.ts +++ b/packages/cli/src/config/extensions/extensionSettings.ts @@ -8,7 +8,7 @@ import * as fs from 'node:fs/promises'; import * as fsSync from 'node:fs'; import * as dotenv from 'dotenv'; -import { ExtensionStorage } from '../extension.js'; +import { ExtensionStorage } from './storage.js'; import type { ExtensionConfig } from '../extension.js'; import prompts from 'prompts'; diff --git a/packages/cli/src/config/extensions/github.test.ts b/packages/cli/src/config/extensions/github.test.ts index 74f6992286..57eaa3e32e 100644 --- a/packages/cli/src/config/extensions/github.test.ts +++ b/packages/cli/src/config/extensions/github.test.ts @@ -4,7 +4,15 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { + afterEach, + beforeEach, + describe, + expect, + it, + vi, + type MockedFunction, +} from 'vitest'; import { checkForExtensionUpdate, cloneFromGit, @@ -22,7 +30,9 @@ import * as path from 'node:path'; import * as tar from 'tar'; import * as archiver from 'archiver'; import type { GeminiCLIExtension } from '@google/gemini-cli-core'; -import { ExtensionEnablementManager } from './extensionEnablement.js'; +import { ExtensionManager } from '../extension-manager.js'; +import { loadSettings } from '../settings.js'; +import type { ExtensionSetting } from './extensionSettings.js'; const mockPlatform = vi.hoisted(() => vi.fn()); const mockArch = vi.hoisted(() => vi.fn()); @@ -134,8 +144,34 @@ describe('git extension helpers', () => { revparse: vi.fn(), }; + let extensionManager: ExtensionManager; + let mockRequestConsent: MockedFunction< + (consent: string) => Promise + >; + let mockPromptForSettings: MockedFunction< + (setting: ExtensionSetting) => Promise + >; + let tempHomeDir: string; + let tempWorkspaceDir: string; + beforeEach(() => { + tempHomeDir = fsSync.mkdtempSync( + path.join(os.tmpdir(), 'gemini-cli-test-home-'), + ); + tempWorkspaceDir = fsSync.mkdtempSync( + path.join(tempHomeDir, 'gemini-cli-test-workspace-'), + ); vi.mocked(simpleGit).mockReturnValue(mockGit as unknown as SimpleGit); + mockRequestConsent = vi.fn(); + mockRequestConsent.mockResolvedValue(true); + mockPromptForSettings = vi.fn(); + mockPromptForSettings.mockResolvedValue(''); + extensionManager = new ExtensionManager({ + workspaceDir: tempWorkspaceDir, + requestConsent: mockRequestConsent, + requestSetting: mockPromptForSettings, + loadedSettings: loadSettings(tempWorkspaceDir), + }); }); it('should return NOT_UPDATABLE for non-git extensions', async () => { @@ -151,10 +187,7 @@ describe('git extension helpers', () => { }, contextFiles: [], }; - const result = await checkForExtensionUpdate( - extension, - new ExtensionEnablementManager(), - ); + const result = await checkForExtensionUpdate(extension, extensionManager); expect(result).toBe(ExtensionUpdateState.NOT_UPDATABLE); }); @@ -172,10 +205,7 @@ describe('git extension helpers', () => { contextFiles: [], }; mockGit.getRemotes.mockResolvedValue([]); - const result = await checkForExtensionUpdate( - extension, - new ExtensionEnablementManager(), - ); + const result = await checkForExtensionUpdate(extension, extensionManager); expect(result).toBe(ExtensionUpdateState.ERROR); }); @@ -198,10 +228,7 @@ describe('git extension helpers', () => { mockGit.listRemote.mockResolvedValue('remote-hash\tHEAD'); mockGit.revparse.mockResolvedValue('local-hash'); - const result = await checkForExtensionUpdate( - extension, - new ExtensionEnablementManager(), - ); + const result = await checkForExtensionUpdate(extension, extensionManager); expect(result).toBe(ExtensionUpdateState.UPDATE_AVAILABLE); }); @@ -224,10 +251,7 @@ describe('git extension helpers', () => { mockGit.listRemote.mockResolvedValue('same-hash\tHEAD'); mockGit.revparse.mockResolvedValue('same-hash'); - const result = await checkForExtensionUpdate( - extension, - new ExtensionEnablementManager(), - ); + const result = await checkForExtensionUpdate(extension, extensionManager); expect(result).toBe(ExtensionUpdateState.UP_TO_DATE); }); @@ -246,10 +270,7 @@ describe('git extension helpers', () => { }; mockGit.getRemotes.mockRejectedValue(new Error('git error')); - const result = await checkForExtensionUpdate( - extension, - new ExtensionEnablementManager(), - ); + const result = await checkForExtensionUpdate(extension, extensionManager); expect(result).toBe(ExtensionUpdateState.ERROR); }); }); diff --git a/packages/cli/src/config/extensions/github.ts b/packages/cli/src/config/extensions/github.ts index 367c80501f..5e5e5cde7d 100644 --- a/packages/cli/src/config/extensions/github.ts +++ b/packages/cli/src/config/extensions/github.ts @@ -16,11 +16,11 @@ import * as os from 'node:os'; import * as https from 'node:https'; import * as fs from 'node:fs'; import * as path from 'node:path'; -import { EXTENSIONS_CONFIG_FILENAME, loadExtension } from '../extension.js'; import * as tar from 'tar'; import extract from 'extract-zip'; import { fetchJson, getGitHubToken } from './github_fetch.js'; -import { type ExtensionEnablementManager } from './extensionEnablement.js'; +import type { ExtensionManager } from '../extension-manager.js'; +import { EXTENSIONS_CONFIG_FILENAME } from './variables.js'; /** * Clones a Git repository to a specified local path. @@ -153,16 +153,11 @@ export async function fetchReleaseFromGithub( export async function checkForExtensionUpdate( extension: GeminiCLIExtension, - extensionEnablementManager: ExtensionEnablementManager, - cwd: string = process.cwd(), + extensionManager: ExtensionManager, ): Promise { const installMetadata = extension.installMetadata; if (installMetadata?.type === 'local') { - const newExtension = loadExtension({ - extensionDir: installMetadata.source, - workspaceDir: cwd, - extensionEnablementManager, - }); + const newExtension = extensionManager.loadExtension(installMetadata.source); if (!newExtension) { debugLogger.error( `Failed to check for update for local extension "${extension.name}". Could not load extension from source path: ${installMetadata.source}`, diff --git a/packages/cli/src/config/extensions/storage.ts b/packages/cli/src/config/extensions/storage.ts new file mode 100644 index 0000000000..583bdd044b --- /dev/null +++ b/packages/cli/src/config/extensions/storage.ts @@ -0,0 +1,47 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as path from 'node:path'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import { + EXTENSION_SETTINGS_FILENAME, + EXTENSIONS_CONFIG_FILENAME, +} from './variables.js'; +import { Storage } from '@google/gemini-cli-core'; + +export class ExtensionStorage { + private readonly extensionName: string; + + constructor(extensionName: string) { + this.extensionName = extensionName; + } + + getExtensionDir(): string { + return path.join( + ExtensionStorage.getUserExtensionsDir(), + this.extensionName, + ); + } + + getConfigPath(): string { + return path.join(this.getExtensionDir(), EXTENSIONS_CONFIG_FILENAME); + } + + getEnvFilePath(): string { + return path.join(this.getExtensionDir(), EXTENSION_SETTINGS_FILENAME); + } + + static getUserExtensionsDir(): string { + return new Storage(os.homedir()).getExtensionsDir(); + } + + static async createTmpDir(): Promise { + return await fs.promises.mkdtemp( + path.join(os.tmpdir(), 'gemini-extension'), + ); + } +} diff --git a/packages/cli/src/config/extensions/update.test.ts b/packages/cli/src/config/extensions/update.test.ts index f0ba5ba982..176e7ad3fa 100644 --- a/packages/cli/src/config/extensions/update.test.ts +++ b/packages/cli/src/config/extensions/update.test.ts @@ -4,21 +4,22 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { vi } from 'vitest'; +import { vi, type MockedFunction } from 'vitest'; import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; -import { - EXTENSIONS_CONFIG_FILENAME, - INSTALL_METADATA_FILENAME, - loadExtension, -} from '../extension.js'; import { checkForAllExtensionUpdates, updateExtension } from './update.js'; import { GEMINI_DIR } from '@google/gemini-cli-core'; import { isWorkspaceTrusted } from '../trustedFolders.js'; import { ExtensionUpdateState } from '../../ui/state/extensions.js'; import { createExtension } from '../../test-utils/createExtension.js'; -import { ExtensionEnablementManager } from './extensionEnablement.js'; +import { + EXTENSIONS_CONFIG_FILENAME, + INSTALL_METADATA_FILENAME, +} from './variables.js'; +import { ExtensionManager } from '../extension-manager.js'; +import { loadSettings } from '../settings.js'; +import type { ExtensionSetting } from './extensionSettings.js'; const mockGit = { clone: vi.fn(), @@ -74,6 +75,11 @@ describe('update tests', () => { let tempHomeDir: string; let tempWorkspaceDir: string; let userExtensionsDir: string; + let extensionManager: ExtensionManager; + let mockRequestConsent: MockedFunction<(consent: string) => Promise>; + let mockPromptForSettings: MockedFunction< + (setting: ExtensionSetting) => Promise + >; beforeEach(() => { tempHomeDir = fs.mkdtempSync( @@ -93,6 +99,16 @@ describe('update tests', () => { }); vi.spyOn(process, 'cwd').mockReturnValue(tempWorkspaceDir); Object.values(mockGit).forEach((fn) => fn.mockReset()); + mockRequestConsent = vi.fn(); + mockRequestConsent.mockResolvedValue(true); + mockPromptForSettings = vi.fn(); + mockPromptForSettings.mockResolvedValue(''); + extensionManager = new ExtensionManager({ + workspaceDir: tempWorkspaceDir, + requestConsent: mockRequestConsent, + requestSetting: mockPromptForSettings, + loadedSettings: loadSettings(tempWorkspaceDir), + }); }); afterEach(() => { @@ -127,17 +143,10 @@ describe('update tests', () => { ); }); mockGit.getRemotes.mockResolvedValue([{ name: 'origin' }]); - const extensionEnablementManager = new ExtensionEnablementManager(); - const extension = loadExtension({ - extensionDir: targetExtDir, - workspaceDir: tempWorkspaceDir, - extensionEnablementManager, - })!; + const extension = extensionManager.loadExtension(targetExtDir)!; const updateInfo = await updateExtension( extension, - extensionEnablementManager, - tempHomeDir, - async (_) => true, + extensionManager, ExtensionUpdateState.UPDATE_AVAILABLE, () => {}, ); @@ -181,17 +190,10 @@ describe('update tests', () => { mockGit.getRemotes.mockResolvedValue([{ name: 'origin' }]); const dispatch = vi.fn(); - const extensionEnablementManager = new ExtensionEnablementManager(); - const extension = loadExtension({ - extensionDir, - workspaceDir: tempWorkspaceDir, - extensionEnablementManager, - })!; + const extension = extensionManager.loadExtension(extensionDir)!; await updateExtension( extension, - extensionEnablementManager, - tempHomeDir, - async (_) => true, + extensionManager, ExtensionUpdateState.UPDATE_AVAILABLE, dispatch, ); @@ -228,18 +230,11 @@ describe('update tests', () => { mockGit.getRemotes.mockResolvedValue([{ name: 'origin' }]); const dispatch = vi.fn(); - const extensionEnablementManager = new ExtensionEnablementManager(); - const extension = loadExtension({ - extensionDir, - workspaceDir: tempWorkspaceDir, - extensionEnablementManager, - })!; + const extension = extensionManager.loadExtension(extensionDir)!; await expect( updateExtension( extension, - extensionEnablementManager, - tempHomeDir, - async (_) => true, + extensionManager, ExtensionUpdateState.UPDATE_AVAILABLE, dispatch, ), @@ -273,12 +268,7 @@ describe('update tests', () => { type: 'git', }, }); - const extensionEnablementManager = new ExtensionEnablementManager(); - const extension = loadExtension({ - extensionDir, - workspaceDir: tempWorkspaceDir, - extensionEnablementManager, - })!; + const extension = extensionManager.loadExtension(extensionDir)!; mockGit.getRemotes.mockResolvedValue([ { name: 'origin', refs: { fetch: 'https://some.git/repo' } }, @@ -289,9 +279,8 @@ describe('update tests', () => { const dispatch = vi.fn(); await checkForAllExtensionUpdates( [extension], - extensionEnablementManager, + extensionManager, dispatch, - tempWorkspaceDir, ); expect(dispatch).toHaveBeenCalledWith({ type: 'SET_STATE', @@ -312,12 +301,7 @@ describe('update tests', () => { type: 'git', }, }); - const extensionEnablementManager = new ExtensionEnablementManager(); - const extension = loadExtension({ - extensionDir, - workspaceDir: tempWorkspaceDir, - extensionEnablementManager, - })!; + const extension = extensionManager.loadExtension(extensionDir)!; mockGit.getRemotes.mockResolvedValue([ { name: 'origin', refs: { fetch: 'https://some.git/repo' } }, @@ -328,9 +312,8 @@ describe('update tests', () => { const dispatch = vi.fn(); await checkForAllExtensionUpdates( [extension], - extensionEnablementManager, + extensionManager, dispatch, - tempWorkspaceDir, ); expect(dispatch).toHaveBeenCalledWith({ type: 'SET_STATE', @@ -355,18 +338,12 @@ describe('update tests', () => { version: '1.0.0', installMetadata: { source: sourceExtensionDir, type: 'local' }, }); - const extensionEnablementManager = new ExtensionEnablementManager(); - const extension = loadExtension({ - extensionDir: installedExtensionDir, - workspaceDir: tempWorkspaceDir, - extensionEnablementManager, - })!; + const extension = extensionManager.loadExtension(installedExtensionDir)!; const dispatch = vi.fn(); await checkForAllExtensionUpdates( [extension], - extensionEnablementManager, + extensionManager, dispatch, - tempWorkspaceDir, ); expect(dispatch).toHaveBeenCalledWith({ type: 'SET_STATE', @@ -391,18 +368,12 @@ describe('update tests', () => { version: '1.0.0', installMetadata: { source: sourceExtensionDir, type: 'local' }, }); - const extensionEnablementManager = new ExtensionEnablementManager(); - const extension = loadExtension({ - extensionDir: installedExtensionDir, - workspaceDir: tempWorkspaceDir, - extensionEnablementManager, - })!; + const extension = extensionManager.loadExtension(installedExtensionDir)!; const dispatch = vi.fn(); await checkForAllExtensionUpdates( [extension], - extensionEnablementManager, + extensionManager, dispatch, - tempWorkspaceDir, ); expect(dispatch).toHaveBeenCalledWith({ type: 'SET_STATE', @@ -423,21 +394,15 @@ describe('update tests', () => { type: 'git', }, }); - const extensionEnablementManager = new ExtensionEnablementManager(); - const extension = loadExtension({ - extensionDir, - workspaceDir: tempWorkspaceDir, - extensionEnablementManager, - })!; + const extension = extensionManager.loadExtension(extensionDir)!; mockGit.getRemotes.mockRejectedValue(new Error('Git error')); const dispatch = vi.fn(); await checkForAllExtensionUpdates( [extension], - extensionEnablementManager, + extensionManager, dispatch, - tempWorkspaceDir, ); expect(dispatch).toHaveBeenCalledWith({ type: 'SET_STATE', diff --git a/packages/cli/src/config/extensions/update.ts b/packages/cli/src/config/extensions/update.ts index 99d80eac5b..141ace88d8 100644 --- a/packages/cli/src/config/extensions/update.ts +++ b/packages/cli/src/config/extensions/update.ts @@ -9,20 +9,13 @@ import { ExtensionUpdateState, type ExtensionUpdateStatus, } from '../../ui/state/extensions.js'; -import { - copyExtension, - installOrUpdateExtension, - loadExtension, - loadInstallMetadata, - ExtensionStorage, - loadExtensionConfig, -} from '../extension.js'; +import { loadInstallMetadata } from '../extension.js'; import { checkForExtensionUpdate } from './github.js'; import { debugLogger, type GeminiCLIExtension } from '@google/gemini-cli-core'; import * as fs from 'node:fs'; import { getErrorMessage } from '../../utils/errors.js'; -import { type ExtensionEnablementManager } from './extensionEnablement.js'; -import { promptForSetting } from './extensionSettings.js'; +import { copyExtension, type ExtensionManager } from '../extension-manager.js'; +import { ExtensionStorage } from './storage.js'; export interface ExtensionUpdateInfo { name: string; @@ -32,9 +25,7 @@ export interface ExtensionUpdateInfo { export async function updateExtension( extension: GeminiCLIExtension, - extensionEnablementManager: ExtensionEnablementManager, - cwd: string = process.cwd(), - requestConsent: (consent: string) => Promise, + extensionManager: ExtensionManager, currentState: ExtensionUpdateState, dispatchExtensionStateUpdate: (action: ExtensionUpdateAction) => void, ): Promise { @@ -67,25 +58,17 @@ export async function updateExtension( const tempDir = await ExtensionStorage.createTmpDir(); try { - const previousExtensionConfig = loadExtensionConfig({ - extensionDir: extension.path, - workspaceDir: cwd, - extensionEnablementManager, - }); - - await installOrUpdateExtension( + const previousExtensionConfig = await extensionManager.loadExtensionConfig( + extension.path, + ); + await extensionManager.installOrUpdateExtension( installMetadata, - requestConsent, - cwd, previousExtensionConfig, - promptForSetting, ); const updatedExtensionStorage = new ExtensionStorage(extension.name); - const updatedExtension = loadExtension({ - extensionDir: updatedExtensionStorage.getExtensionDir(), - workspaceDir: cwd, - extensionEnablementManager, - }); + const updatedExtension = extensionManager.loadExtension( + updatedExtensionStorage.getExtensionDir(), + ); if (!updatedExtension) { dispatchExtensionStateUpdate({ type: 'SET_STATE', @@ -122,11 +105,9 @@ export async function updateExtension( } export async function updateAllUpdatableExtensions( - cwd: string = process.cwd(), - requestConsent: (consent: string) => Promise, extensions: GeminiCLIExtension[], extensionsState: Map, - extensionEnablementManager: ExtensionEnablementManager, + extensionManager: ExtensionManager, dispatch: (action: ExtensionUpdateAction) => void, ): Promise { return ( @@ -140,9 +121,7 @@ export async function updateAllUpdatableExtensions( .map((extension) => updateExtension( extension, - extensionEnablementManager, - cwd, - requestConsent, + extensionManager, extensionsState.get(extension.name)!.status, dispatch, ), @@ -158,9 +137,8 @@ export interface ExtensionUpdateCheckResult { export async function checkForAllExtensionUpdates( extensions: GeminiCLIExtension[], - extensionEnablementManager: ExtensionEnablementManager, + extensionManager: ExtensionManager, dispatch: (action: ExtensionUpdateAction) => void, - cwd: string = process.cwd(), ): Promise { dispatch({ type: 'BATCH_CHECK_START' }); const promises: Array> = []; @@ -183,12 +161,11 @@ export async function checkForAllExtensionUpdates( }, }); promises.push( - checkForExtensionUpdate(extension, extensionEnablementManager, cwd).then( - (state) => - dispatch({ - type: 'SET_STATE', - payload: { name: extension.name, state }, - }), + checkForExtensionUpdate(extension, extensionManager).then((state) => + dispatch({ + type: 'SET_STATE', + payload: { name: extension.name, state }, + }), ), ); } diff --git a/packages/cli/src/config/extensions/variableSchema.ts b/packages/cli/src/config/extensions/variableSchema.ts index 4e0adf3582..0b8abc5021 100644 --- a/packages/cli/src/config/extensions/variableSchema.ts +++ b/packages/cli/src/config/extensions/variableSchema.ts @@ -4,8 +4,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -import type { ExtensionEnablementManager } from './extensionEnablement.js'; - export interface VariableDefinition { type: 'string'; description: string; @@ -17,12 +15,6 @@ export interface VariableSchema { [key: string]: VariableDefinition; } -export interface LoadExtensionContext { - extensionDir: string; - workspaceDir: string; - extensionEnablementManager: ExtensionEnablementManager; -} - const PATH_SEPARATOR_DEFINITION = { type: 'string', description: 'The path separator.', diff --git a/packages/cli/src/config/extensions/variables.ts b/packages/cli/src/config/extensions/variables.ts index 7c6ef84692..78506a9738 100644 --- a/packages/cli/src/config/extensions/variables.ts +++ b/packages/cli/src/config/extensions/variables.ts @@ -4,7 +4,14 @@ * SPDX-License-Identifier: Apache-2.0 */ +import * as path from 'node:path'; import { type VariableSchema, VARIABLE_SCHEMA } from './variableSchema.js'; +import { GEMINI_DIR } from '@google/gemini-cli-core'; + +export const EXTENSIONS_DIRECTORY_NAME = path.join(GEMINI_DIR, 'extensions'); +export const EXTENSIONS_CONFIG_FILENAME = 'gemini-extension.json'; +export const INSTALL_METADATA_FILENAME = '.gemini-extension-install.json'; +export const EXTENSION_SETTINGS_FILENAME = '.env'; export type JsonObject = { [key: string]: JsonValue }; export type JsonArray = JsonValue[]; diff --git a/packages/cli/src/config/settings.test.ts b/packages/cli/src/config/settings.test.ts index 2f7b358f6e..3c79657b14 100644 --- a/packages/cli/src/config/settings.test.ts +++ b/packages/cli/src/config/settings.test.ts @@ -50,7 +50,6 @@ import { import * as fs from 'node:fs'; // fs will be mocked separately import stripJsonComments from 'strip-json-comments'; // Will be mocked separately import { isWorkspaceTrusted } from './trustedFolders.js'; -import { disableExtension, ExtensionStorage } from './extension.js'; // These imports will get the versions from the vi.mock('./settings.js', ...) factory. import { @@ -67,8 +66,8 @@ import { saveSettings, type SettingsFile, } from './settings.js'; -import { FatalConfigError, GEMINI_DIR, Storage } from '@google/gemini-cli-core'; -import { ExtensionEnablementManager } from './extensions/extensionEnablement.js'; +import { FatalConfigError, GEMINI_DIR } from '@google/gemini-cli-core'; +import { ExtensionManager } from './extension-manager.js'; import { updateSettingsFilePreservingFormat } from '../utils/commentJson.js'; const MOCK_WORKSPACE_DIR = '/mock/workspace'; @@ -2392,18 +2391,13 @@ describe('Settings Loading and Merging', () => { describe('migrateDeprecatedSettings', () => { let mockFsExistsSync: Mock; let mockFsReadFileSync: Mock; - let mockDisableExtension: Mock; beforeEach(() => { vi.resetAllMocks(); - mockFsExistsSync = vi.mocked(fs.existsSync); - mockFsReadFileSync = vi.mocked(fs.readFileSync); - mockDisableExtension = vi.mocked(disableExtension); - vi.mocked(ExtensionStorage.getUserExtensionsDir).mockReturnValue( - new Storage(osActual.homedir()).getExtensionsDir(), - ); (mockFsExistsSync as Mock).mockReturnValue(true); + mockFsReadFileSync = vi.mocked(fs.readFileSync); + mockFsReadFileSync.mockReturnValue('{}'); vi.mocked(isWorkspaceTrusted).mockReturnValue({ isTrusted: true, source: undefined, @@ -2438,35 +2432,38 @@ describe('Settings Loading and Merging', () => { const loadedSettings = loadSettings(MOCK_WORKSPACE_DIR); const setValueSpy = vi.spyOn(loadedSettings, 'setValue'); + const extensionManager = new ExtensionManager({ + loadedSettings, + workspaceDir: MOCK_WORKSPACE_DIR, + requestConsent: vi.fn(), + requestSetting: vi.fn(), + }); + const mockDisableExtension = vi.spyOn( + extensionManager, + 'disableExtension', + ); + mockDisableExtension.mockImplementation(() => {}); - migrateDeprecatedSettings(loadedSettings, MOCK_WORKSPACE_DIR); + migrateDeprecatedSettings(loadedSettings, extensionManager); // Check user settings migration expect(mockDisableExtension).toHaveBeenCalledWith( 'user-ext-1', SettingScope.User, - expect.any(ExtensionEnablementManager), - MOCK_WORKSPACE_DIR, ); expect(mockDisableExtension).toHaveBeenCalledWith( 'shared-ext', SettingScope.User, - expect.any(ExtensionEnablementManager), - MOCK_WORKSPACE_DIR, ); // Check workspace settings migration expect(mockDisableExtension).toHaveBeenCalledWith( 'workspace-ext-1', SettingScope.Workspace, - expect.any(ExtensionEnablementManager), - MOCK_WORKSPACE_DIR, ); expect(mockDisableExtension).toHaveBeenCalledWith( 'shared-ext', SettingScope.Workspace, - expect.any(ExtensionEnablementManager), - MOCK_WORKSPACE_DIR, ); // Check that setValue was called to remove the deprecated setting @@ -2508,8 +2505,19 @@ describe('Settings Loading and Merging', () => { const loadedSettings = loadSettings(MOCK_WORKSPACE_DIR); const setValueSpy = vi.spyOn(loadedSettings, 'setValue'); + const extensionManager = new ExtensionManager({ + loadedSettings, + workspaceDir: MOCK_WORKSPACE_DIR, + requestConsent: vi.fn(), + requestSetting: vi.fn(), + }); + const mockDisableExtension = vi.spyOn( + extensionManager, + 'disableExtension', + ); + mockDisableExtension.mockImplementation(() => {}); - migrateDeprecatedSettings(loadedSettings, MOCK_WORKSPACE_DIR); + migrateDeprecatedSettings(loadedSettings, extensionManager); expect(mockDisableExtension).not.toHaveBeenCalled(); expect(setValueSpy).not.toHaveBeenCalled(); diff --git a/packages/cli/src/config/settings.ts b/packages/cli/src/config/settings.ts index 2818c9a214..00cd27a7f7 100644 --- a/packages/cli/src/config/settings.ts +++ b/packages/cli/src/config/settings.ts @@ -32,8 +32,7 @@ import { import { resolveEnvVarsInObject } from '../utils/envVarResolver.js'; import { customDeepMerge, type MergeableObject } from '../utils/deepMerge.js'; import { updateSettingsFilePreservingFormat } from '../utils/commentJson.js'; -import { disableExtension } from './extension.js'; -import { ExtensionEnablementManager } from './extensions/extensionEnablement.js'; +import type { ExtensionManager } from './extension-manager.js'; function getMergeStrategyForPath(path: string[]): MergeStrategy | undefined { let current: SettingDefinition | undefined = undefined; @@ -750,7 +749,7 @@ export function loadSettings( export function migrateDeprecatedSettings( loadedSettings: LoadedSettings, - workspaceDir: string = process.cwd(), + extensionManager: ExtensionManager, ): void { const processScope = (scope: SettingScope) => { const settings = loadedSettings.forScope(scope).settings; @@ -758,14 +757,8 @@ export function migrateDeprecatedSettings( debugLogger.log( `Migrating deprecated extensions.disabled settings from ${scope} settings...`, ); - const extensionEnablementManager = new ExtensionEnablementManager(); for (const extension of settings.extensions.disabled ?? []) { - disableExtension( - extension, - scope, - extensionEnablementManager, - workspaceDir, - ); + extensionManager.disableExtension(extension, scope); } const newExtensionsValue = { ...settings.extensions }; diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index f22a9dda08..05388524f3 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -26,7 +26,6 @@ import { getStartupWarnings } from './utils/startupWarnings.js'; import { getUserStartupWarnings } from './utils/userStartupWarnings.js'; import { ConsolePatcher } from './ui/utils/ConsolePatcher.js'; import { runNonInteractive } from './nonInteractiveCli.js'; -import { loadExtensions } from './config/extension.js'; import { cleanupCheckpoints, registerCleanup, @@ -67,8 +66,9 @@ import { relaunchOnExitCode, } from './utils/relaunch.js'; import { loadSandboxConfig } from './config/sandboxConfig.js'; +import { ExtensionManager } from './config/extension-manager.js'; +import { requestConsentNonInteractive } from './config/extensions/consent.js'; import { createPolicyUpdater } from './config/policy.js'; -import { ExtensionEnablementManager } from './config/extensions/extensionEnablement.js'; export function validateDnsResolutionOrder( order: string | undefined, @@ -225,7 +225,17 @@ export async function startInteractiveUI( export async function main() { setupUnhandledRejectionHandler(); const settings = loadSettings(); - migrateDeprecatedSettings(settings); + migrateDeprecatedSettings( + settings, + // Temporary extension manager only used during this non-interactive UI phase. + new ExtensionManager({ + workspaceDir: process.cwd(), + loadedSettings: settings, + enabledExtensionOverrides: [], + requestConsent: requestConsentNonInteractive, + requestSetting: null, + }), + ); await cleanupCheckpoints(); const argv = await parseArguments(settings.merged); @@ -360,10 +370,17 @@ export async function main() { // to run Gemini CLI. It is now safe to perform expensive initialization that // may have side effects. { - const extensionEnablementManager = new ExtensionEnablementManager( - argv.extensions, - ); - const extensions = loadExtensions(extensionEnablementManager); + // Eventually, `extensions` should move off of `config` entirely and into + // the UI state instead. + const extensionManager = new ExtensionManager({ + loadedSettings: settings, + workspaceDir: process.cwd(), + // At this stage, we still don't have an interactive UI. + requestConsent: requestConsentNonInteractive, + requestSetting: null, + enabledExtensionOverrides: argv.extensions, + }); + const extensions = extensionManager.loadExtensions(); const config = await loadCliConfig( settings.merged, extensions, diff --git a/packages/cli/src/test-utils/createExtension.ts b/packages/cli/src/test-utils/createExtension.ts index 452138e959..f7ad425f06 100644 --- a/packages/cli/src/test-utils/createExtension.ts +++ b/packages/cli/src/test-utils/createExtension.ts @@ -6,14 +6,14 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; -import { - EXTENSIONS_CONFIG_FILENAME, - INSTALL_METADATA_FILENAME, -} from '../config/extension.js'; import { type MCPServerConfig, type ExtensionInstallMetadata, } from '@google/gemini-cli-core'; +import { + EXTENSIONS_CONFIG_FILENAME, + INSTALL_METADATA_FILENAME, +} from '../config/extensions/variables.js'; import type { ExtensionSetting } from '../config/extensions/extensionSettings.js'; export function createExtension({ diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index c8de2a27ec..426543d772 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -93,9 +93,13 @@ import { useMessageQueue } from './hooks/useMessageQueue.js'; import { useAutoAcceptIndicator } from './hooks/useAutoAcceptIndicator.js'; import { useSessionStats } from './contexts/SessionContext.js'; import { useGitBranchName } from './hooks/useGitBranchName.js'; -import { useExtensionUpdates } from './hooks/useExtensionUpdates.js'; +import { + useConfirmUpdateRequests, + useExtensionUpdates, +} from './hooks/useExtensionUpdates.js'; import { ShellFocusContext } from './contexts/ShellFocusContext.js'; -import { ExtensionEnablementManager } from '../config/extensions/extensionEnablement.js'; +import { ExtensionManager } from '../config/extension-manager.js'; +import { requestConsentInteractive } from '../config/extensions/consent.js'; const CTRL_EXIT_PROMPT_DURATION_MS = 1000; const QUEUE_ERROR_DISPLAY_DURATION_MS = 3000; @@ -165,21 +169,28 @@ export const AppContainer = (props: AppContainerProps) => { ); const extensions = config.getExtensions(); - const [extensionEnablementManager] = useState( - new ExtensionEnablementManager(config.getEnabledExtensions()), + const [extensionManager] = useState( + new ExtensionManager({ + enabledExtensionOverrides: config.getEnabledExtensions(), + workspaceDir: config.getWorkingDir(), + requestConsent: (description) => + requestConsentInteractive( + description, + addConfirmUpdateExtensionRequest, + ), + // TODO: Support requesting settings in the interactive CLI + requestSetting: null, + loadedSettings: settings, + }), ); + + const { addConfirmUpdateExtensionRequest, confirmUpdateExtensionRequests } = + useConfirmUpdateRequests(); const { extensionsUpdateState, extensionsUpdateStateInternal, dispatchExtensionStateUpdate, - confirmUpdateExtensionRequests, - addConfirmUpdateExtensionRequest, - } = useExtensionUpdates( - extensions, - extensionEnablementManager, - historyManager.addItem, - config.getWorkingDir(), - ); + } = useExtensionUpdates(extensions, extensionManager, historyManager.addItem); const [isPermissionsDialogOpen, setPermissionsDialogOpen] = useState(false); const openPermissionsDialog = useCallback( diff --git a/packages/cli/src/ui/hooks/useExtensionUpdates.test.ts b/packages/cli/src/ui/hooks/useExtensionUpdates.test.ts index 8270bdf789..b0949035d0 100644 --- a/packages/cli/src/ui/hooks/useExtensionUpdates.test.ts +++ b/packages/cli/src/ui/hooks/useExtensionUpdates.test.ts @@ -8,7 +8,6 @@ import { vi } from 'vitest'; import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; -import { loadExtension } from '../../config/extension.js'; import { createExtension } from '../../test-utils/createExtension.js'; import { useExtensionUpdates } from './useExtensionUpdates.js'; import { GEMINI_DIR, type GeminiCLIExtension } from '@google/gemini-cli-core'; @@ -19,7 +18,8 @@ import { updateExtension, } from '../../config/extensions/update.js'; import { ExtensionUpdateState } from '../state/extensions.js'; -import { ExtensionEnablementManager } from '../../config/extensions/extensionEnablement.js'; +import { ExtensionManager } from '../../config/extension-manager.js'; +import { loadSettings } from '../../config/settings.js'; vi.mock('os', async (importOriginal) => { const mockedOs = await importOriginal(); @@ -36,17 +36,29 @@ vi.mock('../../config/extensions/update.js', () => ({ describe('useExtensionUpdates', () => { let tempHomeDir: string; + let tempWorkspaceDir: string; let userExtensionsDir: string; + let extensionManager: ExtensionManager; beforeEach(() => { tempHomeDir = fs.mkdtempSync( path.join(os.tmpdir(), 'gemini-cli-test-home-'), ); vi.mocked(os.homedir).mockReturnValue(tempHomeDir); + tempWorkspaceDir = fs.mkdtempSync( + path.join(tempHomeDir, 'gemini-cli-test-workspace-'), + ); + vi.spyOn(process, 'cwd').mockReturnValue(tempWorkspaceDir); userExtensionsDir = path.join(tempHomeDir, GEMINI_DIR, 'extensions'); fs.mkdirSync(userExtensionsDir, { recursive: true }); vi.mocked(checkForAllExtensionUpdates).mockReset(); vi.mocked(updateExtension).mockReset(); + extensionManager = new ExtensionManager({ + workspaceDir: tempHomeDir, + requestConsent: vi.fn(), + requestSetting: vi.fn(), + loadedSettings: loadSettings(), + }); }); afterEach(() => { @@ -71,10 +83,9 @@ describe('useExtensionUpdates', () => { }, ]; const addItem = vi.fn(); - const cwd = '/test/cwd'; vi.mocked(checkForAllExtensionUpdates).mockImplementation( - async (_extensions, _extensionEnablementManager, dispatch, _cwd) => { + async (_extensions, _extensionManager, dispatch) => { dispatch({ type: 'SET_STATE', payload: { @@ -88,9 +99,8 @@ describe('useExtensionUpdates', () => { renderHook(() => useExtensionUpdates( extensions as GeminiCLIExtension[], - new ExtensionEnablementManager(), + extensionManager, addItem, - cwd, ), ); @@ -116,17 +126,12 @@ describe('useExtensionUpdates', () => { autoUpdate: true, }, }); - const extensionEnablementManager = new ExtensionEnablementManager(); - const extension = loadExtension({ - extensionDir, - workspaceDir: tempHomeDir, - extensionEnablementManager, - })!; + const extension = extensionManager.loadExtension(extensionDir)!; const addItem = vi.fn(); vi.mocked(checkForAllExtensionUpdates).mockImplementation( - async (_extensions, _extensionEnablementManager, dispatch, _cwd) => { + async (_extensions, _extensionManager, dispatch) => { dispatch({ type: 'SET_STATE', payload: { @@ -144,12 +149,7 @@ describe('useExtensionUpdates', () => { }); renderHook(() => - useExtensionUpdates( - [extension], - extensionEnablementManager, - addItem, - tempHomeDir, - ), + useExtensionUpdates([extension], extensionManager, addItem), ); await waitFor( @@ -188,24 +188,15 @@ describe('useExtensionUpdates', () => { }, }); - const extensionEnablementManager = new ExtensionEnablementManager(); const extensions = [ - loadExtension({ - extensionDir: extensionDir1, - workspaceDir: tempHomeDir, - extensionEnablementManager, - })!, - loadExtension({ - extensionDir: extensionDir2, - workspaceDir: tempHomeDir, - extensionEnablementManager, - })!, + extensionManager.loadExtension(extensionDir1)!, + extensionManager.loadExtension(extensionDir2)!, ]; const addItem = vi.fn(); vi.mocked(checkForAllExtensionUpdates).mockImplementation( - async (_extensions, _extensionEnablementManager, dispatch, _cwd) => { + async (_extensions, _extensionManager, dispatch) => { dispatch({ type: 'SET_STATE', payload: { @@ -236,12 +227,7 @@ describe('useExtensionUpdates', () => { }); renderHook(() => - useExtensionUpdates( - extensions, - extensionEnablementManager, - addItem, - tempHomeDir, - ), + useExtensionUpdates(extensions, extensionManager, addItem), ); await waitFor( @@ -299,10 +285,9 @@ describe('useExtensionUpdates', () => { }, ]; const addItem = vi.fn(); - const cwd = '/test/cwd'; vi.mocked(checkForAllExtensionUpdates).mockImplementation( - async (_extensions, _extensionEnablementManager, dispatch, _cwd) => { + async (_extensions, _extensionManager, dispatch) => { dispatch({ type: 'BATCH_CHECK_START' }); dispatch({ type: 'SET_STATE', @@ -323,13 +308,11 @@ describe('useExtensionUpdates', () => { }, ); - const extensionEnablementManager = new ExtensionEnablementManager(); renderHook(() => useExtensionUpdates( extensions as GeminiCLIExtension[], - extensionEnablementManager, + extensionManager, addItem, - cwd, ), ); diff --git a/packages/cli/src/ui/hooks/useExtensionUpdates.ts b/packages/cli/src/ui/hooks/useExtensionUpdates.ts index 5103cde875..a4e9e2598e 100644 --- a/packages/cli/src/ui/hooks/useExtensionUpdates.ts +++ b/packages/cli/src/ui/hooks/useExtensionUpdates.ts @@ -18,12 +18,9 @@ import { checkForAllExtensionUpdates, updateExtension, } from '../../config/extensions/update.js'; -import { - requestConsentInteractive, - type ExtensionUpdateInfo, -} from '../../config/extension.js'; +import { type ExtensionUpdateInfo } from '../../config/extension.js'; import { checkExhaustive } from '../../utils/checks.js'; -import type { ExtensionEnablementManager } from '../../config/extensions/extensionEnablement.js'; +import type { ExtensionManager } from '../../config/extension-manager.js'; type ConfirmationRequestWrapper = { prompt: React.ReactNode; @@ -48,16 +45,7 @@ function confirmationRequestsReducer( } } -export const useExtensionUpdates = ( - extensions: GeminiCLIExtension[], - extensionEnablementManager: ExtensionEnablementManager, - addItem: UseHistoryManagerReturn['addItem'], - cwd: string, -) => { - const [extensionsUpdateState, dispatchExtensionStateUpdate] = useReducer( - extensionUpdatesReducer, - initialExtensionUpdatesState, - ); +export const useConfirmUpdateRequests = () => { const [ confirmUpdateExtensionRequests, dispatchConfirmUpdateExtensionRequests, @@ -82,6 +70,22 @@ export const useExtensionUpdates = ( }, [dispatchConfirmUpdateExtensionRequests], ); + return { + addConfirmUpdateExtensionRequest, + confirmUpdateExtensionRequests, + dispatchConfirmUpdateExtensionRequests, + }; +}; + +export const useExtensionUpdates = ( + extensions: GeminiCLIExtension[], + extensionManager: ExtensionManager, + addItem: UseHistoryManagerReturn['addItem'], +) => { + const [extensionsUpdateState, dispatchExtensionStateUpdate] = useReducer( + extensionUpdatesReducer, + initialExtensionUpdatesState, + ); useEffect(() => { const extensionsToCheck = extensions.filter((extension) => { @@ -95,15 +99,13 @@ export const useExtensionUpdates = ( if (extensionsToCheck.length === 0) return; checkForAllExtensionUpdates( extensionsToCheck, - extensionEnablementManager, + extensionManager, dispatchExtensionStateUpdate, - cwd, ); }, [ extensions, - extensionEnablementManager, + extensionManager, extensionsUpdateState.extensionStatuses, - cwd, dispatchExtensionStateUpdate, ]); @@ -158,13 +160,7 @@ export const useExtensionUpdates = ( } else { const updatePromise = updateExtension( extension, - extensionEnablementManager, - cwd, - (description) => - requestConsentInteractive( - description, - addConfirmUpdateExtensionRequest, - ), + extensionManager, currentState.status, dispatchExtensionStateUpdate, ); @@ -213,14 +209,7 @@ export const useExtensionUpdates = ( }); }); } - }, [ - extensions, - extensionEnablementManager, - extensionsUpdateState, - addConfirmUpdateExtensionRequest, - addItem, - cwd, - ]); + }, [extensions, extensionManager, extensionsUpdateState, addItem]); const extensionsUpdateStateComputed = useMemo(() => { const result = new Map(); @@ -237,7 +226,5 @@ export const useExtensionUpdates = ( extensionsUpdateState: extensionsUpdateStateComputed, extensionsUpdateStateInternal: extensionsUpdateState.extensionStatuses, dispatchExtensionStateUpdate, - confirmUpdateExtensionRequests, - addConfirmUpdateExtensionRequest, }; }; From 48ff9e1555da32f7919fbb4ed0043441d386ab91 Mon Sep 17 00:00:00 2001 From: shishu314 Date: Thu, 23 Oct 2025 16:46:46 -0400 Subject: [PATCH 05/12] fix(infra) - Fix merge queue skipper issues for chain e2e (#11810) Co-authored-by: gemini-cli-robot --- .github/workflows/test_chained_e2e.yml | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test_chained_e2e.yml b/.github/workflows/test_chained_e2e.yml index ba156e316f..adb77ffa03 100644 --- a/.github/workflows/test_chained_e2e.yml +++ b/.github/workflows/test_chained_e2e.yml @@ -29,6 +29,7 @@ permissions: jobs: merge_queue_skipper: name: 'Merge Queue Skipper' + permissions: 'read-all' runs-on: 'gemini-cli-ubuntu-16-core' outputs: skip: '${{ steps.merge-queue-e2e-skipper.outputs.skip-check }}' @@ -37,6 +38,7 @@ jobs: uses: 'cariad-tech/merge-queue-ci-skipper@1032489e59437862c90a08a2c92809c903883772' # ratchet:cariad-tech/merge-queue-ci-skipper@main with: secret: '${{ secrets.GITHUB_TOKEN }}' + continue-on-error: true download_repo_name: runs-on: 'gemini-cli-ubuntu-16-core' @@ -113,8 +115,12 @@ jobs: e2e_linux: name: 'E2E Test (Linux) - ${{ matrix.sandbox }}' - needs: 'parse_run_context' + needs: + - 'merge_queue_skipper' + - 'parse_run_context' runs-on: 'gemini-cli-ubuntu-16-core' + if: | + always() && (needs.merge_queue_skipper.result !='success' || needs.merge_queue_skipper.outputs.skip == 'false') strategy: fail-fast: false matrix: @@ -161,8 +167,12 @@ jobs: e2e_mac: name: 'E2E Test (macOS)' - needs: 'parse_run_context' + needs: + - 'merge_queue_skipper' + - 'parse_run_context' runs-on: 'macos-latest' + if: | + always() && (needs.merge_queue_skipper.result !='success' || needs.merge_queue_skipper.outputs.skip == 'false') steps: - name: 'Checkout' uses: 'actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955' # ratchet:actions/checkout@v5 @@ -200,7 +210,7 @@ jobs: - 'merge_queue_skipper' - 'parse_run_context' if: | - needs.merge_queue_skipper.outputs.skip == 'false' + always() && (needs.merge_queue_skipper.result !='success' || needs.merge_queue_skipper.outputs.skip == 'false') runs-on: 'gemini-cli-windows-16-core' continue-on-error: true @@ -258,7 +268,7 @@ jobs: e2e: name: 'E2E' if: | - always() && needs.merge_queue_skipper.outputs.skip == 'false' + always() && (needs.merge_queue_skipper.result !='success' || needs.merge_queue_skipper.outputs.skip == 'false') needs: - 'e2e_linux' - 'e2e_mac' From 5e70a7dd461d817dcc8e26aecf41c82111752d13 Mon Sep 17 00:00:00 2001 From: cornmander Date: Thu, 23 Oct 2025 16:55:01 -0400 Subject: [PATCH 06/12] fix: align shell allowlist handling (#11510) (#11813) --- integration-tests/run_shell_command.test.ts | 51 +++++++++++++++++++++ packages/core/src/tools/shell.test.ts | 12 +++++ packages/core/src/tools/shell.ts | 18 ++++++-- 3 files changed, 76 insertions(+), 5 deletions(-) diff --git a/integration-tests/run_shell_command.test.ts b/integration-tests/run_shell_command.test.ts index bfd0484c92..c71f6239ed 100644 --- a/integration-tests/run_shell_command.test.ts +++ b/integration-tests/run_shell_command.test.ts @@ -61,6 +61,28 @@ function getDisallowedFileReadCommand(testFile: string): { } } +function getChainedEchoCommand(): { allowPattern: string; command: string } { + const secondCommand = getAllowedListCommand(); + switch (shell) { + case 'powershell': + return { + allowPattern: 'Write-Output', + command: `Write-Output "foo" && ${secondCommand}`, + }; + case 'cmd': + return { + allowPattern: 'echo', + command: `echo "foo" && ${secondCommand}`, + }; + case 'bash': + default: + return { + allowPattern: 'echo', + command: `echo "foo" && ${secondCommand}`, + }; + } +} + describe('run_shell_command', () => { it('should be able to run a shell command', async () => { const rig = new TestRig(); @@ -405,6 +427,35 @@ describe('run_shell_command', () => { expect(failureLog!.toolRequest.success).toBe(false); }); + it('should reject chained commands when only the first segment is allowlisted in non-interactive mode', async () => { + const rig = new TestRig(); + await rig.setup( + 'should reject chained commands when only the first segment is allowlisted', + ); + + const chained = getChainedEchoCommand(); + const shellInjection = `!{${chained.command}}`; + + await rig.run( + { + stdin: `${shellInjection}\n`, + yolo: false, + }, + `--allowed-tools=ShellTool(${chained.allowPattern})`, + ); + + // CLI should refuse to execute the chained command without scheduling run_shell_command. + const toolLogs = rig + .readToolLogs() + .filter((log) => log.toolRequest.name === 'run_shell_command'); + + // Success is false because tool is in the scheduled state. + for (const log of toolLogs) { + expect(log.toolRequest.success).toBe(false); + expect(log.toolRequest.args).toContain('&&'); + } + }); + it('should allow all with "ShellTool" and other specific tools', async () => { const rig = new TestRig(); await rig.setup( diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index 21f380b66c..4ba6dd8353 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -480,6 +480,18 @@ describe('ShellTool', () => { invocation.shouldConfirmExecute(new AbortController().signal), ).rejects.toThrow('wc'); }); + + it('should require all segments of a chained command to be allowlisted', async () => { + (mockConfig.getAllowedTools as Mock).mockReturnValue([ + 'ShellTool(echo)', + ]); + const invocation = shellTool.build({ command: 'echo "foo" && ls -l' }); + await expect( + invocation.shouldConfirmExecute(new AbortController().signal), + ).rejects.toThrow( + 'Command "echo "foo" && ls -l" is not in the list of allowed tools for non-interactive mode.', + ); + }); }); }); diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index a224fd6d37..ed7269cec7 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -9,6 +9,7 @@ import path from 'node:path'; import os, { EOL } from 'node:os'; import crypto from 'node:crypto'; import type { Config } from '../config/config.js'; +import type { AnyToolInvocation } from '../index.js'; import { ToolErrorType } from './tool-error.js'; import type { ToolInvocation, @@ -36,10 +37,9 @@ import { getCommandRoots, initializeShellParsers, isCommandAllowed, - SHELL_TOOL_NAMES, + isShellInvocationAllowlisted, stripShellWrapper, } from '../utils/shell-utils.js'; -import { doesToolInvocationMatch } from '../utils/tool-utils.js'; import { SHELL_TOOL_NAME } from './tool-names.js'; export const OUTPUT_UPDATE_INTERVAL_MS = 1000; @@ -90,9 +90,7 @@ export class ShellToolInvocation extends BaseToolInvocation< !this.config.isInteractive() && this.config.getApprovalMode() !== ApprovalMode.YOLO ) { - const allowedTools = this.config.getAllowedTools() || []; - const [SHELL_TOOL_NAME] = SHELL_TOOL_NAMES; - if (doesToolInvocationMatch(SHELL_TOOL_NAME, command, allowedTools)) { + if (this.isInvocationAllowlisted(command)) { // If it's an allowed shell command, we don't need to confirm execution. return false; } @@ -324,6 +322,16 @@ export class ShellToolInvocation extends BaseToolInvocation< } } } + + private isInvocationAllowlisted(command: string): boolean { + const allowedTools = this.config.getAllowedTools() || []; + if (allowedTools.length === 0) { + return false; + } + + const invocation = { params: { command } } as unknown as AnyToolInvocation; + return isShellInvocationAllowlisted(invocation, allowedTools); + } } function getShellToolDescription(): string { From aa6ae954efeab1beb2b1a41ccd5d39c204bd728d Mon Sep 17 00:00:00 2001 From: Tommaso Sciortino Date: Thu, 23 Oct 2025 14:41:21 -0700 Subject: [PATCH 07/12] Use raw writes to stdin where possible in tests (#11837) --- .../src/ui/contexts/KeypressContext.test.tsx | 548 ++++-------------- packages/cli/src/ui/hooks/useKeypress.test.ts | 97 +--- 2 files changed, 122 insertions(+), 523 deletions(-) diff --git a/packages/cli/src/ui/contexts/KeypressContext.test.tsx b/packages/cli/src/ui/contexts/KeypressContext.test.tsx index b4f3b17074..719d938c62 100644 --- a/packages/cli/src/ui/contexts/KeypressContext.test.tsx +++ b/packages/cli/src/ui/contexts/KeypressContext.test.tsx @@ -31,33 +31,29 @@ vi.mock('ink', async (importOriginal) => { }; }); +const PASTE_START = '\x1B[200~'; +const PASTE_END = '\x1B[201~'; + class MockStdin extends EventEmitter { isTTY = true; setRawMode = vi.fn(); override on = this.addListener; override removeListener = super.removeListener; - write = vi.fn(); resume = vi.fn(); pause = vi.fn(); - // Helper to simulate a keypress event + write(text: string) { + this.emit('data', Buffer.from(text)); + } + + /** + * Used to directly simulate keyPress events. Certain keypress events might + * be impossible to fire in certain versions of node. This allows us to + * sidestep readline entirely and just emit the keypress we want. + */ pressKey(key: Partial) { this.emit('keypress', null, key); } - - // Helper to simulate a kitty protocol sequence - sendKittySequence(sequence: string) { - this.emit('data', Buffer.from(sequence)); - } - - // Helper to simulate a paste event - sendPaste(text: string) { - const PASTE_MODE_PREFIX = `\x1b[200~`; - const PASTE_MODE_SUFFIX = `\x1b[201~`; - this.emit('data', Buffer.from(PASTE_MODE_PREFIX)); - this.emit('data', Buffer.from(text)); - this.emit('data', Buffer.from(PASTE_MODE_SUFFIX)); - } } describe('KeypressContext - Kitty Protocol', () => { @@ -94,13 +90,11 @@ describe('KeypressContext - Kitty Protocol', () => { wrapper({ children, kittyProtocolEnabled: true }), }); - act(() => { - result.current.subscribe(keyHandler); - }); + act(() => result.current.subscribe(keyHandler)); // Send kitty protocol sequence for regular enter: ESC[13u act(() => { - stdin.sendKittySequence(`\x1b[13u`); + stdin.write(`\x1b[13u`); }); expect(keyHandler).toHaveBeenCalledWith( @@ -122,13 +116,11 @@ describe('KeypressContext - Kitty Protocol', () => { wrapper({ children, kittyProtocolEnabled: true }), }); - act(() => { - result.current.subscribe(keyHandler); - }); + act(() => result.current.subscribe(keyHandler)); // Send kitty protocol sequence for numpad enter: ESC[57414u act(() => { - stdin.sendKittySequence(`\x1b[57414u`); + stdin.write(`\x1b[57414u`); }); expect(keyHandler).toHaveBeenCalledWith( @@ -150,13 +142,11 @@ describe('KeypressContext - Kitty Protocol', () => { wrapper({ children, kittyProtocolEnabled: true }), }); - act(() => { - result.current.subscribe(keyHandler); - }); + act(() => result.current.subscribe(keyHandler)); // Send kitty protocol sequence for numpad enter with Shift (modifier 2): ESC[57414;2u act(() => { - stdin.sendKittySequence(`\x1b[57414;2u`); + stdin.write(`\x1b[57414;2u`); }); expect(keyHandler).toHaveBeenCalledWith( @@ -178,13 +168,11 @@ describe('KeypressContext - Kitty Protocol', () => { wrapper({ children, kittyProtocolEnabled: true }), }); - act(() => { - result.current.subscribe(keyHandler); - }); + act(() => result.current.subscribe(keyHandler)); // Send kitty protocol sequence for numpad enter with Ctrl (modifier 5): ESC[57414;5u act(() => { - stdin.sendKittySequence(`\x1b[57414;5u`); + stdin.write(`\x1b[57414;5u`); }); expect(keyHandler).toHaveBeenCalledWith( @@ -206,13 +194,11 @@ describe('KeypressContext - Kitty Protocol', () => { wrapper({ children, kittyProtocolEnabled: true }), }); - act(() => { - result.current.subscribe(keyHandler); - }); + act(() => result.current.subscribe(keyHandler)); // Send kitty protocol sequence for numpad enter with Alt (modifier 3): ESC[57414;3u act(() => { - stdin.sendKittySequence(`\x1b[57414;3u`); + stdin.write(`\x1b[57414;3u`); }); expect(keyHandler).toHaveBeenCalledWith( @@ -234,13 +220,11 @@ describe('KeypressContext - Kitty Protocol', () => { wrapper({ children, kittyProtocolEnabled: false }), }); - act(() => { - result.current.subscribe(keyHandler); - }); + act(() => result.current.subscribe(keyHandler)); // Send kitty protocol sequence for numpad enter act(() => { - stdin.sendKittySequence(`\x1b[57414u`); + stdin.write(`\x1b[57414u`); }); // When kitty protocol is disabled, the sequence should be passed through @@ -263,13 +247,11 @@ describe('KeypressContext - Kitty Protocol', () => { wrapper({ children, kittyProtocolEnabled: true }), }); - act(() => { - result.current.subscribe(keyHandler); - }); + act(() => result.current.subscribe(keyHandler)); // Send kitty protocol sequence for escape: ESC[27u act(() => { - stdin.sendKittySequence('\x1b[27u'); + stdin.write('\x1b[27u'); }); expect(keyHandler).toHaveBeenCalledWith( @@ -288,7 +270,7 @@ describe('KeypressContext - Kitty Protocol', () => { act(() => result.current.subscribe(keyHandler)); act(() => { - stdin.sendKittySequence(`\x1b[9u`); + stdin.write(`\x1b[9u`); }); expect(keyHandler).toHaveBeenCalledWith( @@ -307,7 +289,7 @@ describe('KeypressContext - Kitty Protocol', () => { // Modifier 2 is Shift act(() => { - stdin.sendKittySequence(`\x1b[9;2u`); + stdin.write(`\x1b[9;2u`); }); expect(keyHandler).toHaveBeenCalledWith( @@ -325,7 +307,7 @@ describe('KeypressContext - Kitty Protocol', () => { act(() => result.current.subscribe(keyHandler)); act(() => { - stdin.sendKittySequence(`\x1b[127u`); + stdin.write(`\x1b[127u`); }); expect(keyHandler).toHaveBeenCalledWith( @@ -344,7 +326,7 @@ describe('KeypressContext - Kitty Protocol', () => { // Modifier 3 is Alt/Option act(() => { - stdin.sendKittySequence(`\x1b[127;3u`); + stdin.write(`\x1b[127;3u`); }); expect(keyHandler).toHaveBeenCalledWith( @@ -363,7 +345,7 @@ describe('KeypressContext - Kitty Protocol', () => { // Modifier 5 is Ctrl act(() => { - stdin.sendKittySequence(`\x1b[127;5u`); + stdin.write(`\x1b[127;5u`); }); expect(keyHandler).toHaveBeenCalledWith( @@ -385,13 +367,13 @@ describe('KeypressContext - Kitty Protocol', () => { wrapper, }); - act(() => { - result.current.subscribe(keyHandler); - }); + act(() => result.current.subscribe(keyHandler)); // Simulate a bracketed paste event act(() => { - stdin.sendPaste(pastedText); + stdin.write(PASTE_START); + stdin.write(pastedText); + stdin.write(PASTE_END); }); await waitFor(() => { @@ -437,13 +419,11 @@ describe('KeypressContext - Kitty Protocol', () => { const { result } = renderHook(() => useKeypressContext(), { wrapper }); - act(() => { - result.current.subscribe(keyHandler); - }); + act(() => result.current.subscribe(keyHandler)); // Send a kitty sequence act(() => { - stdin.sendKittySequence('\x1b[27u'); + stdin.write('\x1b[27u'); }); expect(keyHandler).toHaveBeenCalled(); @@ -466,14 +446,10 @@ describe('KeypressContext - Kitty Protocol', () => { const { result } = renderHook(() => useKeypressContext(), { wrapper }); - act(() => { - result.current.subscribe(keyHandler); - }); + act(() => result.current.subscribe(keyHandler)); // Send a complete kitty sequence for escape - act(() => { - stdin.sendKittySequence('\x1b[27u'); - }); + act(() => stdin.write('\x1b[27u')); expect(consoleLogSpy).toHaveBeenCalledWith( '[DEBUG] Kitty buffer accumulating:', @@ -502,15 +478,11 @@ describe('KeypressContext - Kitty Protocol', () => { const { result } = renderHook(() => useKeypressContext(), { wrapper }); - act(() => { - result.current.subscribe(keyHandler); - }); + act(() => result.current.subscribe(keyHandler)); // Send a long sequence starting with a valid kitty prefix to trigger overflow const longSequence = '\x1b[1;' + '1'.repeat(100); - act(() => { - stdin.sendKittySequence(longSequence); - }); + act(() => stdin.write(longSequence)); expect(consoleLogSpy).toHaveBeenCalledWith( '[DEBUG] Kitty buffer overflow, clearing:', @@ -532,31 +504,13 @@ describe('KeypressContext - Kitty Protocol', () => { const { result } = renderHook(() => useKeypressContext(), { wrapper }); - act(() => { - result.current.subscribe(keyHandler); - }); + act(() => result.current.subscribe(keyHandler)); // Send incomplete kitty sequence - act(() => { - stdin.pressKey({ - name: undefined, - ctrl: false, - meta: false, - shift: false, - sequence: '\x1b[1', - }); - }); + act(() => stdin.pressKey({ sequence: '\x1b[1' })); // Send Ctrl+C - act(() => { - stdin.pressKey({ - name: 'c', - ctrl: true, - meta: false, - shift: false, - sequence: '\x03', - }); - }); + act(() => stdin.write('\x03')); expect(consoleLogSpy).toHaveBeenCalledWith( '[DEBUG] Kitty buffer cleared on Ctrl+C:', @@ -586,21 +540,11 @@ describe('KeypressContext - Kitty Protocol', () => { const { result } = renderHook(() => useKeypressContext(), { wrapper }); - act(() => { - result.current.subscribe(keyHandler); - }); + act(() => result.current.subscribe(keyHandler)); // Send incomplete kitty sequence const sequence = '\x1b[12'; - act(() => { - stdin.pressKey({ - name: undefined, - ctrl: false, - meta: false, - shift: false, - sequence, - }); - }); + act(() => stdin.pressKey({ sequence })); // Verify debug logging for accumulation expect(consoleLogSpy).toHaveBeenCalledWith( @@ -629,6 +573,9 @@ describe('KeypressContext - Kitty Protocol', () => { { sequence: `\x1b[1~`, expected: { name: 'home' } }, { sequence: `\x1b[4~`, expected: { name: 'end' } }, { sequence: `\x1b[2~`, expected: { name: 'insert' } }, + // Reverse tabs + { sequence: `\x1b[Z`, expected: { name: 'tab', shift: true } }, + { sequence: `\x1b[1;2Z`, expected: { name: 'tab', shift: true } }, // Legacy Arrows { sequence: `\x1b[A`, @@ -662,7 +609,7 @@ describe('KeypressContext - Kitty Protocol', () => { const { result } = renderHook(() => useKeypressContext(), { wrapper }); act(() => result.current.subscribe(keyHandler)); - act(() => stdin.sendKittySequence(sequence)); + act(() => stdin.write(sequence)); expect(keyHandler).toHaveBeenCalledWith( expect.objectContaining(expected), @@ -671,33 +618,14 @@ describe('KeypressContext - Kitty Protocol', () => { ); }); - describe('Shift+Tab forms', () => { - it.each([ - { sequence: `\x1b[Z`, description: 'legacy reverse Tab' }, - { sequence: `\x1b[1;2Z`, description: 'parameterized reverse Tab' }, - ])( - 'should recognize $description "$sequence" as Shift+Tab', - ({ sequence }) => { - const keyHandler = vi.fn(); - const { result } = renderHook(() => useKeypressContext(), { wrapper }); - act(() => result.current.subscribe(keyHandler)); - - act(() => stdin.sendKittySequence(sequence)); - expect(keyHandler).toHaveBeenCalledWith( - expect.objectContaining({ name: 'tab', shift: true }), - ); - }, - ); - }); - describe('Double-tap and batching', () => { it('should emit two delete events for double-tap CSI[3~', async () => { const keyHandler = vi.fn(); const { result } = renderHook(() => useKeypressContext(), { wrapper }); act(() => result.current.subscribe(keyHandler)); - act(() => stdin.sendKittySequence(`\x1b[3~`)); - act(() => stdin.sendKittySequence(`\x1b[3~`)); + act(() => stdin.write(`\x1b[3~`)); + act(() => stdin.write(`\x1b[3~`)); expect(keyHandler).toHaveBeenNthCalledWith( 1, @@ -714,7 +642,7 @@ describe('KeypressContext - Kitty Protocol', () => { const { result } = renderHook(() => useKeypressContext(), { wrapper }); act(() => result.current.subscribe(keyHandler)); - act(() => stdin.sendKittySequence(`\x1b[3~\x1b[5~`)); + act(() => stdin.write(`\x1b[3~\x1b[5~`)); expect(keyHandler).toHaveBeenCalledWith( expect.objectContaining({ name: 'delete' }), @@ -732,15 +660,9 @@ describe('KeypressContext - Kitty Protocol', () => { // Incomplete ESC sequence then a complete Delete act(() => { // Provide an incomplete ESC sequence chunk with a real ESC character - stdin.pressKey({ - name: undefined, - ctrl: false, - meta: false, - shift: false, - sequence: '\x1b[1;', - }); + stdin.write('\x1b[1;'); }); - act(() => stdin.sendKittySequence(`\x1b[3~`)); + act(() => stdin.write(`\x1b[3~`)); expect(keyHandler).toHaveBeenCalledTimes(1); expect(keyHandler).toHaveBeenCalledWith( @@ -786,20 +708,9 @@ describe('Drag and Drop Handling', () => { const { result } = renderHook(() => useKeypressContext(), { wrapper }); - act(() => { - result.current.subscribe(keyHandler); - }); + act(() => result.current.subscribe(keyHandler)); - act(() => { - stdin.pressKey({ - name: undefined, - ctrl: false, - meta: false, - shift: false, - paste: false, - sequence: SINGLE_QUOTE, - }); - }); + act(() => stdin.write(SINGLE_QUOTE)); expect(keyHandler).not.toHaveBeenCalled(); }); @@ -809,20 +720,9 @@ describe('Drag and Drop Handling', () => { const { result } = renderHook(() => useKeypressContext(), { wrapper }); - act(() => { - result.current.subscribe(keyHandler); - }); + act(() => result.current.subscribe(keyHandler)); - act(() => { - stdin.pressKey({ - name: undefined, - ctrl: false, - meta: false, - shift: false, - paste: false, - sequence: DOUBLE_QUOTE, - }); - }); + act(() => stdin.write(DOUBLE_QUOTE)); expect(keyHandler).not.toHaveBeenCalled(); }); @@ -834,33 +734,13 @@ describe('Drag and Drop Handling', () => { const { result } = renderHook(() => useKeypressContext(), { wrapper }); - act(() => { - result.current.subscribe(keyHandler); - }); + act(() => result.current.subscribe(keyHandler)); // Start by single quote - act(() => { - stdin.pressKey({ - name: undefined, - ctrl: false, - meta: false, - shift: false, - paste: false, - sequence: SINGLE_QUOTE, - }); - }); + act(() => stdin.write(SINGLE_QUOTE)); // Send single character - act(() => { - stdin.pressKey({ - name: undefined, - ctrl: false, - meta: false, - shift: false, - paste: false, - sequence: 'a', - }); - }); + act(() => stdin.write('a')); // Character should not be immediately broadcast expect(keyHandler).not.toHaveBeenCalled(); @@ -885,66 +765,16 @@ describe('Drag and Drop Handling', () => { const { result } = renderHook(() => useKeypressContext(), { wrapper }); - act(() => { - result.current.subscribe(keyHandler); - }); + act(() => result.current.subscribe(keyHandler)); // Start by single quote - act(() => { - stdin.pressKey({ - name: undefined, - ctrl: false, - meta: false, - shift: false, - paste: false, - sequence: SINGLE_QUOTE, - }); - }); + act(() => stdin.write(SINGLE_QUOTE)); // Send multiple characters - act(() => { - stdin.pressKey({ - name: undefined, - ctrl: false, - meta: false, - shift: false, - paste: false, - sequence: 'p', - }); - }); - - act(() => { - stdin.pressKey({ - name: undefined, - ctrl: false, - meta: false, - shift: false, - paste: false, - sequence: 'a', - }); - }); - - act(() => { - stdin.pressKey({ - name: undefined, - ctrl: false, - meta: false, - shift: false, - paste: false, - sequence: 't', - }); - }); - - act(() => { - stdin.pressKey({ - name: undefined, - ctrl: false, - meta: false, - shift: false, - paste: false, - sequence: 'h', - }); - }); + act(() => stdin.write('p')); + act(() => stdin.write('a')); + act(() => stdin.write('t')); + act(() => stdin.write('h')); // Characters should not be immediately broadcast expect(keyHandler).not.toHaveBeenCalled(); @@ -1101,7 +931,7 @@ describe('Kitty Sequence Parsing', () => { act(() => result.current.subscribe(keyHandler)); if (kittySequence) { - act(() => stdin.sendKittySequence(kittySequence)); + act(() => stdin.write(kittySequence)); } else if (input) { act(() => stdin.pressKey(input)); } @@ -1126,16 +956,7 @@ describe('Kitty Sequence Parsing', () => { const { result } = renderHook(() => useKeypressContext(), { wrapper }); act(() => result.current.subscribe(keyHandler)); - act(() => - stdin.pressKey({ - name: undefined, - ctrl: false, - meta: false, - shift: false, - paste: false, - sequence: '\\', - }), - ); + act(() => stdin.write('\\')); // Advance timers to trigger the backslash timeout act(() => { @@ -1155,29 +976,16 @@ describe('Kitty Sequence Parsing', () => { const keyHandler = vi.fn(); const { result } = renderHook(() => useKeypressContext(), { wrapper }); - act(() => { - result.current.subscribe(keyHandler); - }); + act(() => result.current.subscribe(keyHandler)); // Send incomplete kitty sequence - act(() => { - stdin.pressKey({ - name: undefined, - ctrl: false, - meta: false, - shift: false, - paste: false, - sequence: '\x1b[1;', - }); - }); + act(() => stdin.pressKey({ sequence: '\x1b[1;' })); // Should not broadcast immediately expect(keyHandler).not.toHaveBeenCalled(); // Advance time just before timeout - act(() => { - vi.advanceTimersByTime(KITTY_SEQUENCE_TIMEOUT_MS - 5); - }); + act(() => vi.advanceTimersByTime(KITTY_SEQUENCE_TIMEOUT_MS - 5)); // Still shouldn't broadcast expect(keyHandler).not.toHaveBeenCalled(); @@ -1201,22 +1009,11 @@ describe('Kitty Sequence Parsing', () => { const keyHandler = vi.fn(); const { result } = renderHook(() => useKeypressContext(), { wrapper }); - act(() => { - result.current.subscribe(keyHandler); - }); + act(() => result.current.subscribe(keyHandler)); // Send a CSI sequence that doesn't match kitty patterns // ESC[m is SGR reset, not a kitty sequence - act(() => { - stdin.pressKey({ - name: undefined, - ctrl: false, - meta: false, - shift: false, - paste: false, - sequence: '\x1b[m', - }); - }); + act(() => stdin.write('\x1b[m')); // Should broadcast immediately as it's not a valid kitty pattern expect(keyHandler).toHaveBeenCalledWith( @@ -1232,21 +1029,10 @@ describe('Kitty Sequence Parsing', () => { const keyHandler = vi.fn(); const { result } = renderHook(() => useKeypressContext(), { wrapper }); - act(() => { - result.current.subscribe(keyHandler); - }); + act(() => result.current.subscribe(keyHandler)); // Send complete kitty sequence for Ctrl+A - act(() => { - stdin.pressKey({ - name: undefined, - ctrl: false, - meta: false, - shift: false, - paste: false, - sequence: '\x1b[97;5u', - }); - }); + act(() => stdin.write('\x1b[97;5u')); // Should parse and broadcast immediately expect(keyHandler).toHaveBeenCalledWith( @@ -1262,21 +1048,10 @@ describe('Kitty Sequence Parsing', () => { const keyHandler = vi.fn(); const { result } = renderHook(() => useKeypressContext(), { wrapper }); - act(() => { - result.current.subscribe(keyHandler); - }); + act(() => result.current.subscribe(keyHandler)); - // Send multiple kitty sequences at once - act(() => { - stdin.pressKey({ - name: undefined, - ctrl: false, - meta: false, - shift: false, - paste: false, - sequence: '\x1b[97;5u\x1b[98;5u', // Ctrl+a followed by Ctrl+b - }); - }); + // Send Ctrl+a followed by Ctrl+b + act(() => stdin.write('\x1b[97;5u\x1b[98;5u')); // Should parse both sequences expect(keyHandler).toHaveBeenCalledTimes(2); @@ -1302,38 +1077,16 @@ describe('Kitty Sequence Parsing', () => { const keyHandler = vi.fn(); const { result } = renderHook(() => useKeypressContext(), { wrapper }); - act(() => { - result.current.subscribe(keyHandler); - }); + act(() => result.current.subscribe(keyHandler)); // Send incomplete kitty sequence - act(() => { - stdin.pressKey({ - name: undefined, - ctrl: false, - meta: false, - shift: false, - paste: false, - sequence: '\x1b[1;', - }); - }); + act(() => stdin.pressKey({ sequence: '\x1b[1;' })); // Press Ctrl+C - act(() => { - stdin.pressKey({ - name: 'c', - ctrl: true, - meta: false, - shift: false, - paste: false, - sequence: '\x03', - }); - }); + act(() => stdin.write('\x03')); // Advance past timeout - act(() => { - vi.advanceTimersByTime(KITTY_SEQUENCE_TIMEOUT_MS + 10); - }); + act(() => vi.advanceTimersByTime(KITTY_SEQUENCE_TIMEOUT_MS + 10)); // Should only have received Ctrl+C, not the incomplete sequence expect(keyHandler).toHaveBeenCalledTimes(1); @@ -1349,21 +1102,11 @@ describe('Kitty Sequence Parsing', () => { const keyHandler = vi.fn(); const { result } = renderHook(() => useKeypressContext(), { wrapper }); - act(() => { - result.current.subscribe(keyHandler); - }); + act(() => result.current.subscribe(keyHandler)); // Send valid kitty sequence followed by invalid CSI - act(() => { - stdin.pressKey({ - name: undefined, - ctrl: false, - meta: false, - shift: false, - paste: false, - sequence: '\x1b[13u\x1b[!', // Valid enter, then invalid sequence - }); - }); + // Valid enter, then invalid sequence + act(() => stdin.write('\x1b[13u\x1b[!')); // Should parse valid sequence and flush invalid immediately expect(keyHandler).toHaveBeenCalledTimes(2); @@ -1390,21 +1133,10 @@ describe('Kitty Sequence Parsing', () => { wrapper({ children, kittyProtocolEnabled: false }), }); - act(() => { - result.current.subscribe(keyHandler); - }); + act(() => result.current.subscribe(keyHandler)); // Send what would be a kitty sequence - act(() => { - stdin.pressKey({ - name: undefined, - ctrl: false, - meta: false, - shift: false, - paste: false, - sequence: '\x1b[13u', - }); - }); + act(() => stdin.write('\x1b[13u')); // Should pass through without parsing expect(keyHandler).toHaveBeenCalledWith( @@ -1454,58 +1186,25 @@ describe('Kitty Sequence Parsing', () => { const keyHandler = vi.fn(); const { result } = renderHook(() => useKeypressContext(), { wrapper }); - act(() => { - result.current.subscribe(keyHandler); - }); + act(() => result.current.subscribe(keyHandler)); // Start incomplete sequence - act(() => { - stdin.pressKey({ - name: undefined, - ctrl: false, - meta: false, - shift: false, - paste: false, - sequence: '\x1b[1', - }); - }); + act(() => stdin.pressKey({ sequence: '\x1b[1' })); // Advance time partway - act(() => { - vi.advanceTimersByTime(30); - }); + act(() => vi.advanceTimersByTime(30)); // Add more to sequence - act(() => { - stdin.pressKey({ - name: undefined, - ctrl: false, - meta: false, - shift: false, - paste: false, - sequence: '3', - }); - }); + act(() => stdin.write('3')); // Advance time from the first timeout point - act(() => { - vi.advanceTimersByTime(25); - }); + act(() => vi.advanceTimersByTime(25)); // Should not have timed out yet (timeout restarted) expect(keyHandler).not.toHaveBeenCalled(); // Complete the sequence - act(() => { - stdin.pressKey({ - name: undefined, - ctrl: false, - meta: false, - shift: false, - paste: false, - sequence: 'u', - }); - }); + act(() => stdin.write('u')); // Should now parse as complete enter key expect(keyHandler).toHaveBeenCalledWith( @@ -1520,27 +1219,16 @@ describe('Kitty Sequence Parsing', () => { const keyHandler = vi.fn(); const { result } = renderHook(() => useKeypressContext(), { wrapper }); - act(() => { - result.current.subscribe(keyHandler); - }); + act(() => result.current.subscribe(keyHandler)); // Send incomplete kitty sequence - act(() => { - stdin.pressKey({ - sequence: '\x1b[1;', - }); - }); + act(() => stdin.pressKey({ sequence: '\x1b[1;' })); // Incomplete sequence should be buffered, not broadcast expect(keyHandler).not.toHaveBeenCalled(); // Send FOCUS_IN event - const FOCUS_IN = '\x1b[I'; - act(() => { - stdin.pressKey({ - sequence: FOCUS_IN, - }); - }); + act(() => stdin.write('\x1b[I')); // The buffered sequence should be flushed expect(keyHandler).toHaveBeenCalledTimes(1); @@ -1557,27 +1245,16 @@ describe('Kitty Sequence Parsing', () => { const keyHandler = vi.fn(); const { result } = renderHook(() => useKeypressContext(), { wrapper }); - act(() => { - result.current.subscribe(keyHandler); - }); + act(() => result.current.subscribe(keyHandler)); // Send incomplete kitty sequence - act(() => { - stdin.pressKey({ - sequence: '\x1b[1;', - }); - }); + act(() => stdin.pressKey({ sequence: '\x1b[1;' })); // Incomplete sequence should be buffered, not broadcast expect(keyHandler).not.toHaveBeenCalled(); // Send FOCUS_OUT event - const FOCUS_OUT = '\x1b[O'; - act(() => { - stdin.pressKey({ - sequence: FOCUS_OUT, - }); - }); + act(() => stdin.write('\x1b[O')); // The buffered sequence should be flushed expect(keyHandler).toHaveBeenCalledTimes(1); @@ -1595,25 +1272,16 @@ describe('Kitty Sequence Parsing', () => { const keyHandler = vi.fn(); const { result } = renderHook(() => useKeypressContext(), { wrapper }); - act(() => { - result.current.subscribe(keyHandler); - }); + act(() => result.current.subscribe(keyHandler)); // Send incomplete kitty sequence - act(() => { - stdin.pressKey({ - sequence: '\x1b[1;', - }); - }); + act(() => stdin.pressKey({ sequence: '\x1b[1;' })); // Incomplete sequence should be buffered, not broadcast expect(keyHandler).not.toHaveBeenCalled(); // Send paste start sequence - const PASTE_MODE_PREFIX = `\x1b[200~`; - act(() => { - stdin.emit('data', Buffer.from(PASTE_MODE_PREFIX)); - }); + act(() => stdin.write(`\x1b[200~`)); // The buffered sequence should be flushed expect(keyHandler).toHaveBeenCalledTimes(1); @@ -1629,13 +1297,11 @@ describe('Kitty Sequence Parsing', () => { const pastedText = 'hello'; const PASTE_MODE_SUFFIX = `\x1b[201~`; act(() => { - stdin.emit('data', Buffer.from(pastedText)); - stdin.emit('data', Buffer.from(PASTE_MODE_SUFFIX)); + stdin.write(pastedText); + stdin.write(PASTE_MODE_SUFFIX); }); - act(() => { - vi.runAllTimers(); - }); + act(() => vi.runAllTimers()); // The paste event should be broadcast expect(keyHandler).toHaveBeenCalledTimes(2); diff --git a/packages/cli/src/ui/hooks/useKeypress.test.ts b/packages/cli/src/ui/hooks/useKeypress.test.ts index 495946efb5..243152cc42 100644 --- a/packages/cli/src/ui/hooks/useKeypress.test.ts +++ b/packages/cli/src/ui/hooks/useKeypress.test.ts @@ -6,12 +6,10 @@ import React from 'react'; import { renderHook, act } from '@testing-library/react'; -import type { Key } from './useKeypress.js'; import { useKeypress } from './useKeypress.js'; import { KeypressProvider } from '../contexts/KeypressContext.js'; import { useStdin } from 'ink'; import { EventEmitter } from 'node:events'; -import { PassThrough } from 'node:stream'; import type { Mock } from 'vitest'; // Mock the 'ink' module to control stdin @@ -23,35 +21,8 @@ vi.mock('ink', async (importOriginal) => { }; }); -// Mock the 'readline' module -vi.mock('readline', () => { - const mockedReadline = { - createInterface: vi.fn().mockReturnValue({ close: vi.fn() }), - // The paste workaround involves replacing stdin with a PassThrough stream. - // This mock ensures that when emitKeypressEvents is called on that - // stream, we simulate the 'keypress' events that the hook expects. - emitKeypressEvents: vi.fn((stream: EventEmitter) => { - if (stream instanceof PassThrough) { - stream.on('data', (data) => { - const str = data.toString(); - for (const char of str) { - stream.emit('keypress', null, { - name: char, - sequence: char, - ctrl: false, - meta: false, - shift: false, - }); - } - }); - } - }), - }; - return { - ...mockedReadline, - default: mockedReadline, - }; -}); +const PASTE_START = '\x1B[200~'; +const PASTE_END = '\x1B[201~'; class MockStdin extends EventEmitter { isTTY = true; @@ -59,45 +30,11 @@ class MockStdin extends EventEmitter { setRawMode = vi.fn(); override on = this.addListener; override removeListener = super.removeListener; - write = vi.fn(); resume = vi.fn(); + pause = vi.fn(); - private isLegacy = false; - - setLegacy(isLegacy: boolean) { - this.isLegacy = isLegacy; - } - - // Helper to simulate a full paste event. - paste(text: string) { - if (this.isLegacy) { - const PASTE_START = '\x1B[200~'; - const PASTE_END = '\x1B[201~'; - this.emit('data', Buffer.from(`${PASTE_START}${text}${PASTE_END}`)); - } else { - this.emit('keypress', null, { name: 'paste-start' }); - this.emit('keypress', null, { sequence: text }); - this.emit('keypress', null, { name: 'paste-end' }); - } - } - - // Helper to simulate the start of a paste, without the end. - startPaste(text: string) { - if (this.isLegacy) { - this.emit('data', Buffer.from('\x1B[200~' + text)); - } else { - this.emit('keypress', null, { name: 'paste-start' }); - this.emit('keypress', null, { sequence: text }); - } - } - - // Helper to simulate a single keypress event. - pressKey(key: Partial) { - if (this.isLegacy) { - this.emit('data', Buffer.from(key.sequence ?? '')); - } else { - this.emit('keypress', null, key); - } + write(text: string) { + this.emit('data', Buffer.from(text)); } } @@ -140,7 +77,7 @@ describe('useKeypress', () => { renderHook(() => useKeypress(onKeypress, { isActive: false }), { wrapper, }); - act(() => stdin.pressKey({ name: 'a' })); + act(() => stdin.write('a')); expect(onKeypress).not.toHaveBeenCalled(); }); @@ -152,7 +89,7 @@ describe('useKeypress', () => { { key: { name: 'down', sequence: '\x1b[B' } }, ])('should listen for keypress when active for key $key.name', ({ key }) => { renderHook(() => useKeypress(onKeypress, { isActive: true }), { wrapper }); - act(() => stdin.pressKey(key)); + act(() => stdin.write(key.sequence)); expect(onKeypress).toHaveBeenCalledWith(expect.objectContaining(key)); }); @@ -172,14 +109,14 @@ describe('useKeypress', () => { { wrapper }, ); unmount(); - act(() => stdin.pressKey({ name: 'a' })); + act(() => stdin.write('a')); expect(onKeypress).not.toHaveBeenCalled(); }); it('should correctly identify alt+enter (meta key)', () => { renderHook(() => useKeypress(onKeypress, { isActive: true }), { wrapper }); const key = { name: 'return', sequence: '\x1B\r' }; - act(() => stdin.pressKey(key)); + act(() => stdin.write(key.sequence)); expect(onKeypress).toHaveBeenCalledWith( expect.objectContaining({ ...key, meta: true, paste: false }), ); @@ -189,12 +126,10 @@ describe('useKeypress', () => { { description: 'Modern Node (>= v20)', setup: () => setNodeVersion('20.0.0'), - isLegacy: false, }, { description: 'Legacy Node (< v20)', setup: () => setNodeVersion('18.0.0'), - isLegacy: true, }, { description: 'Workaround Env Var', @@ -202,12 +137,10 @@ describe('useKeypress', () => { setNodeVersion('20.0.0'); vi.stubEnv('PASTE_WORKAROUND', 'true'); }, - isLegacy: true, }, - ])('in $description', ({ setup, isLegacy }) => { + ])('in $description', ({ setup }) => { beforeEach(() => { setup(); - stdin.setLegacy(isLegacy); }); it('should process a paste as a single event', () => { @@ -215,7 +148,7 @@ describe('useKeypress', () => { wrapper, }); const pasteText = 'hello world'; - act(() => stdin.paste(pasteText)); + act(() => stdin.write(PASTE_START + pasteText + PASTE_END)); expect(onKeypress).toHaveBeenCalledTimes(1); expect(onKeypress).toHaveBeenCalledWith({ @@ -234,19 +167,19 @@ describe('useKeypress', () => { }); const keyA = { name: 'a', sequence: 'a' }; - act(() => stdin.pressKey(keyA)); + act(() => stdin.write('a')); expect(onKeypress).toHaveBeenCalledWith( expect.objectContaining({ ...keyA, paste: false }), ); const pasteText = 'pasted'; - act(() => stdin.paste(pasteText)); + act(() => stdin.write(PASTE_START + pasteText + PASTE_END)); expect(onKeypress).toHaveBeenCalledWith( expect.objectContaining({ paste: true, sequence: pasteText }), ); const keyB = { name: 'b', sequence: 'b' }; - act(() => stdin.pressKey(keyB)); + act(() => stdin.write('b')); expect(onKeypress).toHaveBeenCalledWith( expect.objectContaining({ ...keyB, paste: false }), ); @@ -261,7 +194,7 @@ describe('useKeypress', () => { ); const pasteText = 'incomplete paste'; - act(() => stdin.startPaste(pasteText)); + act(() => stdin.write(PASTE_START + pasteText)); // No event should be fired yet. expect(onKeypress).not.toHaveBeenCalled(); From 9814f86a2540096eeec0c7121aff380fe92d0c36 Mon Sep 17 00:00:00 2001 From: Riddhi Dutta Date: Fri, 24 Oct 2025 03:17:06 +0530 Subject: [PATCH 08/12] Added parameterization to base-storage-token.test and prompts.test.ts (#11821) --- .../core/__snapshots__/prompts.test.ts.snap | 297 ++++++++++++++++++ packages/core/src/core/prompts.test.ts | 208 ++++-------- .../token-storage/base-token-storage.test.ts | 184 +++++------ 3 files changed, 434 insertions(+), 255 deletions(-) diff --git a/packages/core/src/core/__snapshots__/prompts.test.ts.snap b/packages/core/src/core/__snapshots__/prompts.test.ts.snap index 694e87c78a..9ce812639d 100644 --- a/packages/core/src/core/__snapshots__/prompts.test.ts.snap +++ b/packages/core/src/core/__snapshots__/prompts.test.ts.snap @@ -610,6 +610,303 @@ You are running outside of a sandbox container, directly on the user's system. F +# Final Reminder +Your core function is efficient and safe assistance. Balance extreme conciseness with the crucial need for clarity, especially regarding safety and potential system modifications. Always prioritize user control and project conventions. Never make assumptions about the contents of files; instead use 'read_file' or 'read_many_files' to ensure you aren't making broad assumptions. Finally, you are an agent - please keep going until the user's query is completely resolved." +`; + +exports[`Core System Prompt (prompts.ts) > should return the base prompt when 'no userMemory is provided' 1`] = ` +"You are an interactive CLI agent specializing in software engineering tasks. Your primary goal is to help users safely and efficiently, adhering strictly to the following instructions and utilizing your available tools. + +# Core Mandates + +- **Conventions:** Rigorously adhere to existing project conventions when reading or modifying code. Analyze surrounding code, tests, and configuration first. +- **Libraries/Frameworks:** NEVER assume a library/framework is available or appropriate. Verify its established usage within the project (check imports, configuration files like 'package.json', 'Cargo.toml', 'requirements.txt', 'build.gradle', etc., or observe neighboring files) before employing it. +- **Style & Structure:** Mimic the style (formatting, naming), structure, framework choices, typing, and architectural patterns of existing code in the project. +- **Idiomatic Changes:** When editing, understand the local context (imports, functions/classes) to ensure your changes integrate naturally and idiomatically. +- **Comments:** Add code comments sparingly. Focus on *why* something is done, especially for complex logic, rather than *what* is done. Only add high-value comments if necessary for clarity or if requested by the user. Do not edit comments that are separate from the code you are changing. *NEVER* talk to the user or describe your changes through comments. +- **Proactiveness:** Fulfill the user's request thoroughly. When adding features or fixing bugs, this includes adding tests to ensure quality. Consider all created files, especially tests, to be permanent artifacts unless the user says otherwise. +- **Confirm Ambiguity/Expansion:** Do not take significant actions beyond the clear scope of the request without confirming with the user. If asked *how* to do something, explain first, don't just do it. +- **Explaining Changes:** After completing a code modification or file operation *do not* provide summaries unless asked. +- **Path Construction:** Before using any file system tool (e.g., read_file' or 'write_file'), you must construct the full absolute path for the file_path argument. Always combine the absolute path of the project's root directory with the file's path relative to the root. For example, if the project root is /path/to/project/ and the file is foo/bar/baz.txt, the final path you must use is /path/to/project/foo/bar/baz.txt. If the user provides a relative path, you must resolve it against the root directory to create an absolute path. +- **Do Not revert changes:** Do not revert changes to the codebase unless asked to do so by the user. Only revert changes made by you if they have resulted in an error or if the user has explicitly asked you to revert the changes. + + +# Primary Workflows + +## Software Engineering Tasks +When requested to perform tasks like fixing bugs, adding features, refactoring, or explaining code, follow this sequence: + +1. **Understand:** Think about the user's request and the relevant codebase context. Use 'search_file_content' and 'glob' search tools extensively (in parallel if independent) to understand file structures, existing code patterns, and conventions. Use 'read_file' and 'read_many_files' to understand context and validate any assumptions you may have. +2. **Plan:** Build a coherent and grounded (based on the understanding in step 1) plan for how you intend to resolve the user's task. Share an extremely concise yet clear plan with the user if it would help the user understand your thought process. As part of the plan, you should use an iterative development process that includes writing unit tests to verify your changes. Use output logs or debug statements as part of this process to arrive at a solution. +3. **Implement:** Use the available tools (e.g., 'replace', 'write_file' 'run_shell_command' ...) to act on the plan, strictly adhering to the project's established conventions (detailed under 'Core Mandates'). +4. **Verify (Tests):** If applicable and feasible, verify the changes using the project's testing procedures. Identify the correct test commands and frameworks by examining 'README' files, build/package configuration (e.g., 'package.json'), or existing test execution patterns. NEVER assume standard test commands. +5. **Verify (Standards):** VERY IMPORTANT: After making code changes, execute the project-specific build, linting and type-checking commands (e.g., 'tsc', 'npm run lint', 'ruff check .') that you have identified for this project (or obtained from the user). This ensures code quality and adherence to standards. If unsure about these commands, you can ask the user if they'd like you to run them and if so how to. +6. **Finalize:** After all verification passes, consider the task complete. Do not remove or revert any changes or created files (like tests). Await the user's next instruction. + +## New Applications + +**Goal:** Autonomously implement and deliver a visually appealing, substantially complete, and functional prototype. Utilize all tools at your disposal to implement the application. Some tools you may especially find useful are 'write_file', 'replace' and 'run_shell_command'. + +1. **Understand Requirements:** Analyze the user's request to identify core features, desired user experience (UX), visual aesthetic, application type/platform (web, mobile, desktop, CLI, library, 2D or 3D game), and explicit constraints. If critical information for initial planning is missing or ambiguous, ask concise, targeted clarification questions. +2. **Propose Plan:** Formulate an internal development plan. Present a clear, concise, high-level summary to the user. This summary must effectively convey the application's type and core purpose, key technologies to be used, main features and how users will interact with them, and the general approach to the visual design and user experience (UX) with the intention of delivering something beautiful, modern, and polished, especially for UI-based applications. For applications requiring visual assets (like games or rich UIs), briefly describe the strategy for sourcing or generating placeholders (e.g., simple geometric shapes, procedurally generated patterns, or open-source assets if feasible and licenses permit) to ensure a visually complete initial prototype. Ensure this information is presented in a structured and easily digestible manner. + - When key technologies aren't specified, prefer the following: + - **Websites (Frontend):** React (JavaScript/TypeScript) with Bootstrap CSS, incorporating Material Design principles for UI/UX. + - **Back-End APIs:** Node.js with Express.js (JavaScript/TypeScript) or Python with FastAPI. + - **Full-stack:** Next.js (React/Node.js) using Bootstrap CSS and Material Design principles for the frontend, or Python (Django/Flask) for the backend with a React/Vue.js frontend styled with Bootstrap CSS and Material Design principles. + - **CLIs:** Python or Go. + - **Mobile App:** Compose Multiplatform (Kotlin Multiplatform) or Flutter (Dart) using Material Design libraries and principles, when sharing code between Android and iOS. Jetpack Compose (Kotlin JVM) with Material Design principles or SwiftUI (Swift) for native apps targeted at either Android or iOS, respectively. + - **3d Games:** HTML/CSS/JavaScript with Three.js. + - **2d Games:** HTML/CSS/JavaScript. +3. **User Approval:** Obtain user approval for the proposed plan. +4. **Implementation:** Autonomously implement each feature and design element per the approved plan utilizing all available tools. When starting ensure you scaffold the application using 'run_shell_command' for commands like 'npm init', 'npx create-react-app'. Aim for full scope completion. Proactively create or source necessary placeholder assets (e.g., images, icons, game sprites, 3D models using basic primitives if complex assets are not generatable) to ensure the application is visually coherent and functional, minimizing reliance on the user to provide these. If the model can generate simple assets (e.g., a uniformly colored square sprite, a simple 3D cube), it should do so. Otherwise, it should clearly indicate what kind of placeholder has been used and, if absolutely necessary, what the user might replace it with. Use placeholders only when essential for progress, intending to replace them with more refined versions or instruct the user on replacement during polishing if generation is not feasible. +5. **Verify:** Review work against the original request, the approved plan. Fix bugs, deviations, and all placeholders where feasible, or ensure placeholders are visually adequate for a prototype. Ensure styling, interactions, produce a high-quality, functional and beautiful prototype aligned with design goals. Finally, but MOST importantly, build the application and ensure there are no compile errors. +6. **Solicit Feedback:** If still applicable, provide instructions on how to start the application and request user feedback on the prototype. + +# Operational Guidelines + +## Shell tool output token efficiency: + +IT IS CRITICAL TO FOLLOW THESE GUIDELINES TO AVOID EXCESSIVE TOKEN CONSUMPTION. + +- Always prefer command flags that reduce output verbosity when using 'run_shell_command'. +- Aim to minimize tool output tokens while still capturing necessary information. +- If a command is expected to produce a lot of output, use quiet or silent flags where available and appropriate. +- Always consider the trade-off between output verbosity and the need for information. If a command's full output is essential for understanding the result, avoid overly aggressive quieting that might obscure important details. +- If a command does not have quiet/silent flags or for commands with potentially long output that may not be useful, redirect stdout and stderr to temp files in the project's temporary directory: /tmp/project-temp. For example: 'command > /tmp/project-temp/out.log 2> /tmp/project-temp/err.log'. +- After the command runs, inspect the temp files (e.g. '/tmp/project-temp/out.log' and '/tmp/project-temp/err.log') using commands like 'grep', 'tail', 'head', ... (or platform equivalents). Remove the temp files when done. + + +## Tone and Style (CLI Interaction) +- **Concise & Direct:** Adopt a professional, direct, and concise tone suitable for a CLI environment. +- **Minimal Output:** Aim for fewer than 3 lines of text output (excluding tool use/code generation) per response whenever practical. Focus strictly on the user's query. +- **Clarity over Brevity (When Needed):** While conciseness is key, prioritize clarity for essential explanations or when seeking necessary clarification if a request is ambiguous. +- **No Chitchat:** Avoid conversational filler, preambles ("Okay, I will now..."), or postambles ("I have finished the changes..."). Get straight to the action or answer. +- **Formatting:** Use GitHub-flavored Markdown. Responses will be rendered in monospace. +- **Tools vs. Text:** Use tools for actions, text output *only* for communication. Do not add explanatory comments within tool calls or code blocks unless specifically part of the required code/command itself. +- **Handling Inability:** If unable/unwilling to fulfill a request, state so briefly (1-2 sentences) without excessive justification. Offer alternatives if appropriate. + +## Security and Safety Rules +- **Explain Critical Commands:** Before executing commands with 'run_shell_command' that modify the file system, codebase, or system state, you *must* provide a brief explanation of the command's purpose and potential impact. Prioritize user understanding and safety. You should not ask permission to use the tool; the user will be presented with a confirmation dialogue upon use (you do not need to tell them this). +- **Security First:** Always apply security best practices. Never introduce code that exposes, logs, or commits secrets, API keys, or other sensitive information. + +## Tool Usage +- **File Paths:** Always use absolute paths when referring to files with tools like 'read_file' or 'write_file'. Relative paths are not supported. You must provide an absolute path. +- **Parallelism:** Execute multiple independent tool calls in parallel when feasible (i.e. searching the codebase). +- **Command Execution:** Use the 'run_shell_command' tool for running shell commands, remembering the safety rule to explain modifying commands first. +- **Background Processes:** Use background processes (via \`&\`) for commands that are unlikely to stop on their own, e.g. \`node server.js &\`. If unsure, ask the user. +- **Interactive Commands:** Prefer non-interactive commands when it makes sense; however, some commands are only interactive and expect user input during their execution (e.g. ssh, vim). If you choose to execute an interactive command consider letting the user know they can press \`ctrl + f\` to focus into the shell to provide input. +- **Remembering Facts:** Use the 'save_memory' tool to remember specific, *user-related* facts or preferences when the user explicitly asks, or when they state a clear, concise piece of information that would help personalize or streamline *your future interactions with them* (e.g., preferred coding style, common project paths they use, personal tool aliases). This tool is for user-specific information that should persist across sessions. Do *not* use it for general project context or information. If unsure whether to save something, you can ask the user, "Should I remember that for you?" +- **Respect User Confirmations:** Most tool calls (also denoted as 'function calls') will first require confirmation from the user, where they will either approve or cancel the function call. If a user cancels a function call, respect their choice and do _not_ try to make the function call again. It is okay to request the tool call again _only_ if the user requests that same tool call on a subsequent prompt. When a user cancels a function call, assume best intentions from the user and consider inquiring if they prefer any alternative paths forward. + +## Interaction Details +- **Help Command:** The user can use '/help' to display help information. +- **Feedback:** To report a bug or provide feedback, please use the /bug command. + + +# Outside of Sandbox +You are running outside of a sandbox container, directly on the user's system. For critical commands that are particularly likely to modify the user's system outside of the project directory or system temp directory, as you explain the command to the user (per the Explain Critical Commands rule above), also remind the user to consider enabling sandboxing. + + + + +# Final Reminder +Your core function is efficient and safe assistance. Balance extreme conciseness with the crucial need for clarity, especially regarding safety and potential system modifications. Always prioritize user control and project conventions. Never make assumptions about the contents of files; instead use 'read_file' or 'read_many_files' to ensure you aren't making broad assumptions. Finally, you are an agent - please keep going until the user's query is completely resolved." +`; + +exports[`Core System Prompt (prompts.ts) > should return the base prompt when 'userMemory is empty string' 1`] = ` +"You are an interactive CLI agent specializing in software engineering tasks. Your primary goal is to help users safely and efficiently, adhering strictly to the following instructions and utilizing your available tools. + +# Core Mandates + +- **Conventions:** Rigorously adhere to existing project conventions when reading or modifying code. Analyze surrounding code, tests, and configuration first. +- **Libraries/Frameworks:** NEVER assume a library/framework is available or appropriate. Verify its established usage within the project (check imports, configuration files like 'package.json', 'Cargo.toml', 'requirements.txt', 'build.gradle', etc., or observe neighboring files) before employing it. +- **Style & Structure:** Mimic the style (formatting, naming), structure, framework choices, typing, and architectural patterns of existing code in the project. +- **Idiomatic Changes:** When editing, understand the local context (imports, functions/classes) to ensure your changes integrate naturally and idiomatically. +- **Comments:** Add code comments sparingly. Focus on *why* something is done, especially for complex logic, rather than *what* is done. Only add high-value comments if necessary for clarity or if requested by the user. Do not edit comments that are separate from the code you are changing. *NEVER* talk to the user or describe your changes through comments. +- **Proactiveness:** Fulfill the user's request thoroughly. When adding features or fixing bugs, this includes adding tests to ensure quality. Consider all created files, especially tests, to be permanent artifacts unless the user says otherwise. +- **Confirm Ambiguity/Expansion:** Do not take significant actions beyond the clear scope of the request without confirming with the user. If asked *how* to do something, explain first, don't just do it. +- **Explaining Changes:** After completing a code modification or file operation *do not* provide summaries unless asked. +- **Path Construction:** Before using any file system tool (e.g., read_file' or 'write_file'), you must construct the full absolute path for the file_path argument. Always combine the absolute path of the project's root directory with the file's path relative to the root. For example, if the project root is /path/to/project/ and the file is foo/bar/baz.txt, the final path you must use is /path/to/project/foo/bar/baz.txt. If the user provides a relative path, you must resolve it against the root directory to create an absolute path. +- **Do Not revert changes:** Do not revert changes to the codebase unless asked to do so by the user. Only revert changes made by you if they have resulted in an error or if the user has explicitly asked you to revert the changes. + + +# Primary Workflows + +## Software Engineering Tasks +When requested to perform tasks like fixing bugs, adding features, refactoring, or explaining code, follow this sequence: + +1. **Understand:** Think about the user's request and the relevant codebase context. Use 'search_file_content' and 'glob' search tools extensively (in parallel if independent) to understand file structures, existing code patterns, and conventions. Use 'read_file' and 'read_many_files' to understand context and validate any assumptions you may have. +2. **Plan:** Build a coherent and grounded (based on the understanding in step 1) plan for how you intend to resolve the user's task. Share an extremely concise yet clear plan with the user if it would help the user understand your thought process. As part of the plan, you should use an iterative development process that includes writing unit tests to verify your changes. Use output logs or debug statements as part of this process to arrive at a solution. +3. **Implement:** Use the available tools (e.g., 'replace', 'write_file' 'run_shell_command' ...) to act on the plan, strictly adhering to the project's established conventions (detailed under 'Core Mandates'). +4. **Verify (Tests):** If applicable and feasible, verify the changes using the project's testing procedures. Identify the correct test commands and frameworks by examining 'README' files, build/package configuration (e.g., 'package.json'), or existing test execution patterns. NEVER assume standard test commands. +5. **Verify (Standards):** VERY IMPORTANT: After making code changes, execute the project-specific build, linting and type-checking commands (e.g., 'tsc', 'npm run lint', 'ruff check .') that you have identified for this project (or obtained from the user). This ensures code quality and adherence to standards. If unsure about these commands, you can ask the user if they'd like you to run them and if so how to. +6. **Finalize:** After all verification passes, consider the task complete. Do not remove or revert any changes or created files (like tests). Await the user's next instruction. + +## New Applications + +**Goal:** Autonomously implement and deliver a visually appealing, substantially complete, and functional prototype. Utilize all tools at your disposal to implement the application. Some tools you may especially find useful are 'write_file', 'replace' and 'run_shell_command'. + +1. **Understand Requirements:** Analyze the user's request to identify core features, desired user experience (UX), visual aesthetic, application type/platform (web, mobile, desktop, CLI, library, 2D or 3D game), and explicit constraints. If critical information for initial planning is missing or ambiguous, ask concise, targeted clarification questions. +2. **Propose Plan:** Formulate an internal development plan. Present a clear, concise, high-level summary to the user. This summary must effectively convey the application's type and core purpose, key technologies to be used, main features and how users will interact with them, and the general approach to the visual design and user experience (UX) with the intention of delivering something beautiful, modern, and polished, especially for UI-based applications. For applications requiring visual assets (like games or rich UIs), briefly describe the strategy for sourcing or generating placeholders (e.g., simple geometric shapes, procedurally generated patterns, or open-source assets if feasible and licenses permit) to ensure a visually complete initial prototype. Ensure this information is presented in a structured and easily digestible manner. + - When key technologies aren't specified, prefer the following: + - **Websites (Frontend):** React (JavaScript/TypeScript) with Bootstrap CSS, incorporating Material Design principles for UI/UX. + - **Back-End APIs:** Node.js with Express.js (JavaScript/TypeScript) or Python with FastAPI. + - **Full-stack:** Next.js (React/Node.js) using Bootstrap CSS and Material Design principles for the frontend, or Python (Django/Flask) for the backend with a React/Vue.js frontend styled with Bootstrap CSS and Material Design principles. + - **CLIs:** Python or Go. + - **Mobile App:** Compose Multiplatform (Kotlin Multiplatform) or Flutter (Dart) using Material Design libraries and principles, when sharing code between Android and iOS. Jetpack Compose (Kotlin JVM) with Material Design principles or SwiftUI (Swift) for native apps targeted at either Android or iOS, respectively. + - **3d Games:** HTML/CSS/JavaScript with Three.js. + - **2d Games:** HTML/CSS/JavaScript. +3. **User Approval:** Obtain user approval for the proposed plan. +4. **Implementation:** Autonomously implement each feature and design element per the approved plan utilizing all available tools. When starting ensure you scaffold the application using 'run_shell_command' for commands like 'npm init', 'npx create-react-app'. Aim for full scope completion. Proactively create or source necessary placeholder assets (e.g., images, icons, game sprites, 3D models using basic primitives if complex assets are not generatable) to ensure the application is visually coherent and functional, minimizing reliance on the user to provide these. If the model can generate simple assets (e.g., a uniformly colored square sprite, a simple 3D cube), it should do so. Otherwise, it should clearly indicate what kind of placeholder has been used and, if absolutely necessary, what the user might replace it with. Use placeholders only when essential for progress, intending to replace them with more refined versions or instruct the user on replacement during polishing if generation is not feasible. +5. **Verify:** Review work against the original request, the approved plan. Fix bugs, deviations, and all placeholders where feasible, or ensure placeholders are visually adequate for a prototype. Ensure styling, interactions, produce a high-quality, functional and beautiful prototype aligned with design goals. Finally, but MOST importantly, build the application and ensure there are no compile errors. +6. **Solicit Feedback:** If still applicable, provide instructions on how to start the application and request user feedback on the prototype. + +# Operational Guidelines + +## Shell tool output token efficiency: + +IT IS CRITICAL TO FOLLOW THESE GUIDELINES TO AVOID EXCESSIVE TOKEN CONSUMPTION. + +- Always prefer command flags that reduce output verbosity when using 'run_shell_command'. +- Aim to minimize tool output tokens while still capturing necessary information. +- If a command is expected to produce a lot of output, use quiet or silent flags where available and appropriate. +- Always consider the trade-off between output verbosity and the need for information. If a command's full output is essential for understanding the result, avoid overly aggressive quieting that might obscure important details. +- If a command does not have quiet/silent flags or for commands with potentially long output that may not be useful, redirect stdout and stderr to temp files in the project's temporary directory: /tmp/project-temp. For example: 'command > /tmp/project-temp/out.log 2> /tmp/project-temp/err.log'. +- After the command runs, inspect the temp files (e.g. '/tmp/project-temp/out.log' and '/tmp/project-temp/err.log') using commands like 'grep', 'tail', 'head', ... (or platform equivalents). Remove the temp files when done. + + +## Tone and Style (CLI Interaction) +- **Concise & Direct:** Adopt a professional, direct, and concise tone suitable for a CLI environment. +- **Minimal Output:** Aim for fewer than 3 lines of text output (excluding tool use/code generation) per response whenever practical. Focus strictly on the user's query. +- **Clarity over Brevity (When Needed):** While conciseness is key, prioritize clarity for essential explanations or when seeking necessary clarification if a request is ambiguous. +- **No Chitchat:** Avoid conversational filler, preambles ("Okay, I will now..."), or postambles ("I have finished the changes..."). Get straight to the action or answer. +- **Formatting:** Use GitHub-flavored Markdown. Responses will be rendered in monospace. +- **Tools vs. Text:** Use tools for actions, text output *only* for communication. Do not add explanatory comments within tool calls or code blocks unless specifically part of the required code/command itself. +- **Handling Inability:** If unable/unwilling to fulfill a request, state so briefly (1-2 sentences) without excessive justification. Offer alternatives if appropriate. + +## Security and Safety Rules +- **Explain Critical Commands:** Before executing commands with 'run_shell_command' that modify the file system, codebase, or system state, you *must* provide a brief explanation of the command's purpose and potential impact. Prioritize user understanding and safety. You should not ask permission to use the tool; the user will be presented with a confirmation dialogue upon use (you do not need to tell them this). +- **Security First:** Always apply security best practices. Never introduce code that exposes, logs, or commits secrets, API keys, or other sensitive information. + +## Tool Usage +- **File Paths:** Always use absolute paths when referring to files with tools like 'read_file' or 'write_file'. Relative paths are not supported. You must provide an absolute path. +- **Parallelism:** Execute multiple independent tool calls in parallel when feasible (i.e. searching the codebase). +- **Command Execution:** Use the 'run_shell_command' tool for running shell commands, remembering the safety rule to explain modifying commands first. +- **Background Processes:** Use background processes (via \`&\`) for commands that are unlikely to stop on their own, e.g. \`node server.js &\`. If unsure, ask the user. +- **Interactive Commands:** Prefer non-interactive commands when it makes sense; however, some commands are only interactive and expect user input during their execution (e.g. ssh, vim). If you choose to execute an interactive command consider letting the user know they can press \`ctrl + f\` to focus into the shell to provide input. +- **Remembering Facts:** Use the 'save_memory' tool to remember specific, *user-related* facts or preferences when the user explicitly asks, or when they state a clear, concise piece of information that would help personalize or streamline *your future interactions with them* (e.g., preferred coding style, common project paths they use, personal tool aliases). This tool is for user-specific information that should persist across sessions. Do *not* use it for general project context or information. If unsure whether to save something, you can ask the user, "Should I remember that for you?" +- **Respect User Confirmations:** Most tool calls (also denoted as 'function calls') will first require confirmation from the user, where they will either approve or cancel the function call. If a user cancels a function call, respect their choice and do _not_ try to make the function call again. It is okay to request the tool call again _only_ if the user requests that same tool call on a subsequent prompt. When a user cancels a function call, assume best intentions from the user and consider inquiring if they prefer any alternative paths forward. + +## Interaction Details +- **Help Command:** The user can use '/help' to display help information. +- **Feedback:** To report a bug or provide feedback, please use the /bug command. + + +# Outside of Sandbox +You are running outside of a sandbox container, directly on the user's system. For critical commands that are particularly likely to modify the user's system outside of the project directory or system temp directory, as you explain the command to the user (per the Explain Critical Commands rule above), also remind the user to consider enabling sandboxing. + + + + +# Final Reminder +Your core function is efficient and safe assistance. Balance extreme conciseness with the crucial need for clarity, especially regarding safety and potential system modifications. Always prioritize user control and project conventions. Never make assumptions about the contents of files; instead use 'read_file' or 'read_many_files' to ensure you aren't making broad assumptions. Finally, you are an agent - please keep going until the user's query is completely resolved." +`; + +exports[`Core System Prompt (prompts.ts) > should return the base prompt when 'userMemory is whitespace only' 1`] = ` +"You are an interactive CLI agent specializing in software engineering tasks. Your primary goal is to help users safely and efficiently, adhering strictly to the following instructions and utilizing your available tools. + +# Core Mandates + +- **Conventions:** Rigorously adhere to existing project conventions when reading or modifying code. Analyze surrounding code, tests, and configuration first. +- **Libraries/Frameworks:** NEVER assume a library/framework is available or appropriate. Verify its established usage within the project (check imports, configuration files like 'package.json', 'Cargo.toml', 'requirements.txt', 'build.gradle', etc., or observe neighboring files) before employing it. +- **Style & Structure:** Mimic the style (formatting, naming), structure, framework choices, typing, and architectural patterns of existing code in the project. +- **Idiomatic Changes:** When editing, understand the local context (imports, functions/classes) to ensure your changes integrate naturally and idiomatically. +- **Comments:** Add code comments sparingly. Focus on *why* something is done, especially for complex logic, rather than *what* is done. Only add high-value comments if necessary for clarity or if requested by the user. Do not edit comments that are separate from the code you are changing. *NEVER* talk to the user or describe your changes through comments. +- **Proactiveness:** Fulfill the user's request thoroughly. When adding features or fixing bugs, this includes adding tests to ensure quality. Consider all created files, especially tests, to be permanent artifacts unless the user says otherwise. +- **Confirm Ambiguity/Expansion:** Do not take significant actions beyond the clear scope of the request without confirming with the user. If asked *how* to do something, explain first, don't just do it. +- **Explaining Changes:** After completing a code modification or file operation *do not* provide summaries unless asked. +- **Path Construction:** Before using any file system tool (e.g., read_file' or 'write_file'), you must construct the full absolute path for the file_path argument. Always combine the absolute path of the project's root directory with the file's path relative to the root. For example, if the project root is /path/to/project/ and the file is foo/bar/baz.txt, the final path you must use is /path/to/project/foo/bar/baz.txt. If the user provides a relative path, you must resolve it against the root directory to create an absolute path. +- **Do Not revert changes:** Do not revert changes to the codebase unless asked to do so by the user. Only revert changes made by you if they have resulted in an error or if the user has explicitly asked you to revert the changes. + + +# Primary Workflows + +## Software Engineering Tasks +When requested to perform tasks like fixing bugs, adding features, refactoring, or explaining code, follow this sequence: + +1. **Understand:** Think about the user's request and the relevant codebase context. Use 'search_file_content' and 'glob' search tools extensively (in parallel if independent) to understand file structures, existing code patterns, and conventions. Use 'read_file' and 'read_many_files' to understand context and validate any assumptions you may have. +2. **Plan:** Build a coherent and grounded (based on the understanding in step 1) plan for how you intend to resolve the user's task. Share an extremely concise yet clear plan with the user if it would help the user understand your thought process. As part of the plan, you should use an iterative development process that includes writing unit tests to verify your changes. Use output logs or debug statements as part of this process to arrive at a solution. +3. **Implement:** Use the available tools (e.g., 'replace', 'write_file' 'run_shell_command' ...) to act on the plan, strictly adhering to the project's established conventions (detailed under 'Core Mandates'). +4. **Verify (Tests):** If applicable and feasible, verify the changes using the project's testing procedures. Identify the correct test commands and frameworks by examining 'README' files, build/package configuration (e.g., 'package.json'), or existing test execution patterns. NEVER assume standard test commands. +5. **Verify (Standards):** VERY IMPORTANT: After making code changes, execute the project-specific build, linting and type-checking commands (e.g., 'tsc', 'npm run lint', 'ruff check .') that you have identified for this project (or obtained from the user). This ensures code quality and adherence to standards. If unsure about these commands, you can ask the user if they'd like you to run them and if so how to. +6. **Finalize:** After all verification passes, consider the task complete. Do not remove or revert any changes or created files (like tests). Await the user's next instruction. + +## New Applications + +**Goal:** Autonomously implement and deliver a visually appealing, substantially complete, and functional prototype. Utilize all tools at your disposal to implement the application. Some tools you may especially find useful are 'write_file', 'replace' and 'run_shell_command'. + +1. **Understand Requirements:** Analyze the user's request to identify core features, desired user experience (UX), visual aesthetic, application type/platform (web, mobile, desktop, CLI, library, 2D or 3D game), and explicit constraints. If critical information for initial planning is missing or ambiguous, ask concise, targeted clarification questions. +2. **Propose Plan:** Formulate an internal development plan. Present a clear, concise, high-level summary to the user. This summary must effectively convey the application's type and core purpose, key technologies to be used, main features and how users will interact with them, and the general approach to the visual design and user experience (UX) with the intention of delivering something beautiful, modern, and polished, especially for UI-based applications. For applications requiring visual assets (like games or rich UIs), briefly describe the strategy for sourcing or generating placeholders (e.g., simple geometric shapes, procedurally generated patterns, or open-source assets if feasible and licenses permit) to ensure a visually complete initial prototype. Ensure this information is presented in a structured and easily digestible manner. + - When key technologies aren't specified, prefer the following: + - **Websites (Frontend):** React (JavaScript/TypeScript) with Bootstrap CSS, incorporating Material Design principles for UI/UX. + - **Back-End APIs:** Node.js with Express.js (JavaScript/TypeScript) or Python with FastAPI. + - **Full-stack:** Next.js (React/Node.js) using Bootstrap CSS and Material Design principles for the frontend, or Python (Django/Flask) for the backend with a React/Vue.js frontend styled with Bootstrap CSS and Material Design principles. + - **CLIs:** Python or Go. + - **Mobile App:** Compose Multiplatform (Kotlin Multiplatform) or Flutter (Dart) using Material Design libraries and principles, when sharing code between Android and iOS. Jetpack Compose (Kotlin JVM) with Material Design principles or SwiftUI (Swift) for native apps targeted at either Android or iOS, respectively. + - **3d Games:** HTML/CSS/JavaScript with Three.js. + - **2d Games:** HTML/CSS/JavaScript. +3. **User Approval:** Obtain user approval for the proposed plan. +4. **Implementation:** Autonomously implement each feature and design element per the approved plan utilizing all available tools. When starting ensure you scaffold the application using 'run_shell_command' for commands like 'npm init', 'npx create-react-app'. Aim for full scope completion. Proactively create or source necessary placeholder assets (e.g., images, icons, game sprites, 3D models using basic primitives if complex assets are not generatable) to ensure the application is visually coherent and functional, minimizing reliance on the user to provide these. If the model can generate simple assets (e.g., a uniformly colored square sprite, a simple 3D cube), it should do so. Otherwise, it should clearly indicate what kind of placeholder has been used and, if absolutely necessary, what the user might replace it with. Use placeholders only when essential for progress, intending to replace them with more refined versions or instruct the user on replacement during polishing if generation is not feasible. +5. **Verify:** Review work against the original request, the approved plan. Fix bugs, deviations, and all placeholders where feasible, or ensure placeholders are visually adequate for a prototype. Ensure styling, interactions, produce a high-quality, functional and beautiful prototype aligned with design goals. Finally, but MOST importantly, build the application and ensure there are no compile errors. +6. **Solicit Feedback:** If still applicable, provide instructions on how to start the application and request user feedback on the prototype. + +# Operational Guidelines + +## Shell tool output token efficiency: + +IT IS CRITICAL TO FOLLOW THESE GUIDELINES TO AVOID EXCESSIVE TOKEN CONSUMPTION. + +- Always prefer command flags that reduce output verbosity when using 'run_shell_command'. +- Aim to minimize tool output tokens while still capturing necessary information. +- If a command is expected to produce a lot of output, use quiet or silent flags where available and appropriate. +- Always consider the trade-off between output verbosity and the need for information. If a command's full output is essential for understanding the result, avoid overly aggressive quieting that might obscure important details. +- If a command does not have quiet/silent flags or for commands with potentially long output that may not be useful, redirect stdout and stderr to temp files in the project's temporary directory: /tmp/project-temp. For example: 'command > /tmp/project-temp/out.log 2> /tmp/project-temp/err.log'. +- After the command runs, inspect the temp files (e.g. '/tmp/project-temp/out.log' and '/tmp/project-temp/err.log') using commands like 'grep', 'tail', 'head', ... (or platform equivalents). Remove the temp files when done. + + +## Tone and Style (CLI Interaction) +- **Concise & Direct:** Adopt a professional, direct, and concise tone suitable for a CLI environment. +- **Minimal Output:** Aim for fewer than 3 lines of text output (excluding tool use/code generation) per response whenever practical. Focus strictly on the user's query. +- **Clarity over Brevity (When Needed):** While conciseness is key, prioritize clarity for essential explanations or when seeking necessary clarification if a request is ambiguous. +- **No Chitchat:** Avoid conversational filler, preambles ("Okay, I will now..."), or postambles ("I have finished the changes..."). Get straight to the action or answer. +- **Formatting:** Use GitHub-flavored Markdown. Responses will be rendered in monospace. +- **Tools vs. Text:** Use tools for actions, text output *only* for communication. Do not add explanatory comments within tool calls or code blocks unless specifically part of the required code/command itself. +- **Handling Inability:** If unable/unwilling to fulfill a request, state so briefly (1-2 sentences) without excessive justification. Offer alternatives if appropriate. + +## Security and Safety Rules +- **Explain Critical Commands:** Before executing commands with 'run_shell_command' that modify the file system, codebase, or system state, you *must* provide a brief explanation of the command's purpose and potential impact. Prioritize user understanding and safety. You should not ask permission to use the tool; the user will be presented with a confirmation dialogue upon use (you do not need to tell them this). +- **Security First:** Always apply security best practices. Never introduce code that exposes, logs, or commits secrets, API keys, or other sensitive information. + +## Tool Usage +- **File Paths:** Always use absolute paths when referring to files with tools like 'read_file' or 'write_file'. Relative paths are not supported. You must provide an absolute path. +- **Parallelism:** Execute multiple independent tool calls in parallel when feasible (i.e. searching the codebase). +- **Command Execution:** Use the 'run_shell_command' tool for running shell commands, remembering the safety rule to explain modifying commands first. +- **Background Processes:** Use background processes (via \`&\`) for commands that are unlikely to stop on their own, e.g. \`node server.js &\`. If unsure, ask the user. +- **Interactive Commands:** Prefer non-interactive commands when it makes sense; however, some commands are only interactive and expect user input during their execution (e.g. ssh, vim). If you choose to execute an interactive command consider letting the user know they can press \`ctrl + f\` to focus into the shell to provide input. +- **Remembering Facts:** Use the 'save_memory' tool to remember specific, *user-related* facts or preferences when the user explicitly asks, or when they state a clear, concise piece of information that would help personalize or streamline *your future interactions with them* (e.g., preferred coding style, common project paths they use, personal tool aliases). This tool is for user-specific information that should persist across sessions. Do *not* use it for general project context or information. If unsure whether to save something, you can ask the user, "Should I remember that for you?" +- **Respect User Confirmations:** Most tool calls (also denoted as 'function calls') will first require confirmation from the user, where they will either approve or cancel the function call. If a user cancels a function call, respect their choice and do _not_ try to make the function call again. It is okay to request the tool call again _only_ if the user requests that same tool call on a subsequent prompt. When a user cancels a function call, assume best intentions from the user and consider inquiring if they prefer any alternative paths forward. + +## Interaction Details +- **Help Command:** The user can use '/help' to display help information. +- **Feedback:** To report a bug or provide feedback, please use the /bug command. + + +# Outside of Sandbox +You are running outside of a sandbox container, directly on the user's system. For critical commands that are particularly likely to modify the user's system outside of the project directory or system temp directory, as you explain the command to the user (per the Explain Critical Commands rule above), also remind the user to consider enabling sandboxing. + + + + # Final Reminder Your core function is efficient and safe assistance. Balance extreme conciseness with the crucial need for clarity, especially regarding safety and potential system modifications. Always prioritize user control and project conventions. Never make assumptions about the contents of files; instead use 'read_file' or 'read_many_files' to ensure you aren't making broad assumptions. Finally, you are an agent - please keep going until the user's query is completely resolved." `; diff --git a/packages/core/src/core/prompts.test.ts b/packages/core/src/core/prompts.test.ts index 056b0ce475..013dbfdf5c 100644 --- a/packages/core/src/core/prompts.test.ts +++ b/packages/core/src/core/prompts.test.ts @@ -56,30 +56,17 @@ describe('Core System Prompt (prompts.ts)', () => { } as unknown as Config; }); - it('should return the base prompt when no userMemory is provided', () => { + it.each([ + ['empty string', ''], + ['whitespace only', ' \n \t '], + ])('should return the base prompt when userMemory is %s', (_, userMemory) => { vi.stubEnv('SANDBOX', undefined); - const prompt = getCoreSystemPrompt(mockConfig, ''); + const prompt = getCoreSystemPrompt(mockConfig, userMemory); expect(prompt).not.toContain('---\n\n'); // Separator should not be present expect(prompt).toContain('You are an interactive CLI agent'); // Check for core content expect(prompt).toMatchSnapshot(); // Use snapshot for base prompt structure }); - it('should return the base prompt when userMemory is empty string', () => { - vi.stubEnv('SANDBOX', undefined); - const prompt = getCoreSystemPrompt(mockConfig, ''); - expect(prompt).not.toContain('---\n\n'); - expect(prompt).toContain('You are an interactive CLI agent'); - expect(prompt).toMatchSnapshot(); - }); - - it('should return the base prompt when userMemory is whitespace only', () => { - vi.stubEnv('SANDBOX', undefined); - const prompt = getCoreSystemPrompt(mockConfig, ' \n \t '); - expect(prompt).not.toContain('---\n\n'); - expect(prompt).toContain('You are an interactive CLI agent'); - expect(prompt).toMatchSnapshot(); - }); - it('should append userMemory with separator when provided', () => { vi.stubEnv('SANDBOX', undefined); const memory = 'This is custom user memory.\nBe extra polite.'; @@ -187,19 +174,15 @@ describe('Core System Prompt (prompts.ts)', () => { }); describe('GEMINI_SYSTEM_MD environment variable', () => { - it('should use default prompt when GEMINI_SYSTEM_MD is "false"', () => { - vi.stubEnv('GEMINI_SYSTEM_MD', 'false'); - const prompt = getCoreSystemPrompt(mockConfig); - expect(fs.readFileSync).not.toHaveBeenCalled(); - expect(prompt).not.toContain('custom system prompt'); - }); - - it('should use default prompt when GEMINI_SYSTEM_MD is "0"', () => { - vi.stubEnv('GEMINI_SYSTEM_MD', '0'); - const prompt = getCoreSystemPrompt(mockConfig); - expect(fs.readFileSync).not.toHaveBeenCalled(); - expect(prompt).not.toContain('custom system prompt'); - }); + it.each(['false', '0'])( + 'should use default prompt when GEMINI_SYSTEM_MD is "%s"', + (value) => { + vi.stubEnv('GEMINI_SYSTEM_MD', value); + const prompt = getCoreSystemPrompt(mockConfig); + expect(fs.readFileSync).not.toHaveBeenCalled(); + expect(prompt).not.toContain('custom system prompt'); + }, + ); it('should throw error if GEMINI_SYSTEM_MD points to a non-existent file', () => { const customPath = '/non/existent/path/system.md'; @@ -210,27 +193,19 @@ describe('Core System Prompt (prompts.ts)', () => { ); }); - it('should read from default path when GEMINI_SYSTEM_MD is "true"', () => { - const defaultPath = path.resolve(path.join(GEMINI_DIR, 'system.md')); - vi.stubEnv('GEMINI_SYSTEM_MD', 'true'); - vi.mocked(fs.existsSync).mockReturnValue(true); - vi.mocked(fs.readFileSync).mockReturnValue('custom system prompt'); + it.each(['true', '1'])( + 'should read from default path when GEMINI_SYSTEM_MD is "%s"', + (value) => { + const defaultPath = path.resolve(path.join(GEMINI_DIR, 'system.md')); + vi.stubEnv('GEMINI_SYSTEM_MD', value); + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.readFileSync).mockReturnValue('custom system prompt'); - const prompt = getCoreSystemPrompt(mockConfig); - expect(fs.readFileSync).toHaveBeenCalledWith(defaultPath, 'utf8'); - expect(prompt).toBe('custom system prompt'); - }); - - it('should read from default path when GEMINI_SYSTEM_MD is "1"', () => { - const defaultPath = path.resolve(path.join(GEMINI_DIR, 'system.md')); - vi.stubEnv('GEMINI_SYSTEM_MD', '1'); - vi.mocked(fs.existsSync).mockReturnValue(true); - vi.mocked(fs.readFileSync).mockReturnValue('custom system prompt'); - - const prompt = getCoreSystemPrompt(mockConfig); - expect(fs.readFileSync).toHaveBeenCalledWith(defaultPath, 'utf8'); - expect(prompt).toBe('custom system prompt'); - }); + const prompt = getCoreSystemPrompt(mockConfig); + expect(fs.readFileSync).toHaveBeenCalledWith(defaultPath, 'utf8'); + expect(prompt).toBe('custom system prompt'); + }, + ); it('should read from custom path when GEMINI_SYSTEM_MD provides one, preserving case', () => { const customPath = path.resolve('/custom/path/SyStEm.Md'); @@ -262,37 +237,27 @@ describe('Core System Prompt (prompts.ts)', () => { }); describe('GEMINI_WRITE_SYSTEM_MD environment variable', () => { - it('should not write to file when GEMINI_WRITE_SYSTEM_MD is "false"', () => { - vi.stubEnv('GEMINI_WRITE_SYSTEM_MD', 'false'); - getCoreSystemPrompt(mockConfig); - expect(fs.writeFileSync).not.toHaveBeenCalled(); - }); + it.each(['false', '0'])( + 'should not write to file when GEMINI_WRITE_SYSTEM_MD is "%s"', + (value) => { + vi.stubEnv('GEMINI_WRITE_SYSTEM_MD', value); + getCoreSystemPrompt(mockConfig); + expect(fs.writeFileSync).not.toHaveBeenCalled(); + }, + ); - it('should not write to file when GEMINI_WRITE_SYSTEM_MD is "0"', () => { - vi.stubEnv('GEMINI_WRITE_SYSTEM_MD', '0'); - getCoreSystemPrompt(mockConfig); - expect(fs.writeFileSync).not.toHaveBeenCalled(); - }); - - it('should write to default path when GEMINI_WRITE_SYSTEM_MD is "true"', () => { - const defaultPath = path.resolve(path.join(GEMINI_DIR, 'system.md')); - vi.stubEnv('GEMINI_WRITE_SYSTEM_MD', 'true'); - getCoreSystemPrompt(mockConfig); - expect(fs.writeFileSync).toHaveBeenCalledWith( - defaultPath, - expect.any(String), - ); - }); - - it('should write to default path when GEMINI_WRITE_SYSTEM_MD is "1"', () => { - const defaultPath = path.resolve(path.join(GEMINI_DIR, 'system.md')); - vi.stubEnv('GEMINI_WRITE_SYSTEM_MD', '1'); - getCoreSystemPrompt(mockConfig); - expect(fs.writeFileSync).toHaveBeenCalledWith( - defaultPath, - expect.any(String), - ); - }); + it.each(['true', '1'])( + 'should write to default path when GEMINI_WRITE_SYSTEM_MD is "%s"', + (value) => { + const defaultPath = path.resolve(path.join(GEMINI_DIR, 'system.md')); + vi.stubEnv('GEMINI_WRITE_SYSTEM_MD', value); + getCoreSystemPrompt(mockConfig); + expect(fs.writeFileSync).toHaveBeenCalledWith( + defaultPath, + expect.any(String), + ); + }, + ); it('should write to custom path when GEMINI_WRITE_SYSTEM_MD provides one', () => { const customPath = path.resolve('/custom/path/system.md'); @@ -338,26 +303,12 @@ describe('resolvePathFromEnv helper function', () => { }); describe('when envVar is undefined, empty, or whitespace', () => { - it('should return null for undefined', () => { - const result = resolvePathFromEnv(undefined); - expect(result).toEqual({ - isSwitch: false, - value: null, - isDisabled: false, - }); - }); - - it('should return null for empty string', () => { - const result = resolvePathFromEnv(''); - expect(result).toEqual({ - isSwitch: false, - value: null, - isDisabled: false, - }); - }); - - it('should return null for whitespace only', () => { - const result = resolvePathFromEnv(' \n\t '); + it.each([ + ['undefined', undefined], + ['empty string', ''], + ['whitespace only', ' \n\t '], + ])('should return null for %s', (_, input) => { + const result = resolvePathFromEnv(input); expect(result).toEqual({ isSwitch: false, value: null, @@ -367,52 +318,19 @@ describe('resolvePathFromEnv helper function', () => { }); describe('when envVar is a boolean-like string', () => { - it('should handle "0" as disabled switch', () => { - const result = resolvePathFromEnv('0'); + it.each([ + ['"0" as disabled switch', '0', '0', true], + ['"false" as disabled switch', 'false', 'false', true], + ['"1" as enabled switch', '1', '1', false], + ['"true" as enabled switch', 'true', 'true', false], + ['"FALSE" (case-insensitive)', 'FALSE', 'false', true], + ['"TRUE" (case-insensitive)', 'TRUE', 'true', false], + ])('should handle %s', (_, input, expectedValue, isDisabled) => { + const result = resolvePathFromEnv(input); expect(result).toEqual({ isSwitch: true, - value: '0', - isDisabled: true, - }); - }); - - it('should handle "false" as disabled switch', () => { - const result = resolvePathFromEnv('false'); - expect(result).toEqual({ - isSwitch: true, - value: 'false', - isDisabled: true, - }); - }); - - it('should handle "1" as enabled switch', () => { - const result = resolvePathFromEnv('1'); - expect(result).toEqual({ - isSwitch: true, - value: '1', - isDisabled: false, - }); - }); - - it('should handle "true" as enabled switch', () => { - const result = resolvePathFromEnv('true'); - expect(result).toEqual({ - isSwitch: true, - value: 'true', - isDisabled: false, - }); - }); - - it('should be case-insensitive for boolean values', () => { - expect(resolvePathFromEnv('FALSE')).toEqual({ - isSwitch: true, - value: 'false', - isDisabled: true, - }); - expect(resolvePathFromEnv('TRUE')).toEqual({ - isSwitch: true, - value: 'true', - isDisabled: false, + value: expectedValue, + isDisabled, }); }); }); diff --git a/packages/core/src/mcp/token-storage/base-token-storage.test.ts b/packages/core/src/mcp/token-storage/base-token-storage.test.ts index 1e761d8227..7443ec3bd7 100644 --- a/packages/core/src/mcp/token-storage/base-token-storage.test.ts +++ b/packages/core/src/mcp/token-storage/base-token-storage.test.ts @@ -70,139 +70,103 @@ describe('BaseTokenStorage', () => { expect(() => storage.validateCredentials(credentials)).not.toThrow(); }); - it('should throw for missing server name', () => { - const credentials = { - serverName: '', - token: { - accessToken: 'access-token', - tokenType: 'Bearer', + it.each([ + { + desc: 'missing server name', + credentials: { + serverName: '', + token: { + accessToken: 'access-token', + tokenType: 'Bearer', + }, + updatedAt: Date.now(), }, - updatedAt: Date.now(), - } as OAuthCredentials; - - expect(() => storage.validateCredentials(credentials)).toThrow( - 'Server name is required', - ); - }); - - it('should throw for missing token', () => { - const credentials = { - serverName: 'test-server', - token: null as unknown as OAuthToken, - updatedAt: Date.now(), - } as OAuthCredentials; - - expect(() => storage.validateCredentials(credentials)).toThrow( - 'Token is required', - ); - }); - - it('should throw for missing access token', () => { - const credentials = { - serverName: 'test-server', - token: { - accessToken: '', - tokenType: 'Bearer', + expectedError: 'Server name is required', + }, + { + desc: 'missing token', + credentials: { + serverName: 'test-server', + token: null as unknown as OAuthToken, + updatedAt: Date.now(), }, - updatedAt: Date.now(), - } as OAuthCredentials; - - expect(() => storage.validateCredentials(credentials)).toThrow( - 'Access token is required', - ); - }); - - it('should throw for missing token type', () => { - const credentials = { - serverName: 'test-server', - token: { - accessToken: 'access-token', - tokenType: '', + expectedError: 'Token is required', + }, + { + desc: 'missing access token', + credentials: { + serverName: 'test-server', + token: { + accessToken: '', + tokenType: 'Bearer', + }, + updatedAt: Date.now(), }, - updatedAt: Date.now(), - } as OAuthCredentials; - - expect(() => storage.validateCredentials(credentials)).toThrow( - 'Token type is required', - ); + expectedError: 'Access token is required', + }, + { + desc: 'missing token type', + credentials: { + serverName: 'test-server', + token: { + accessToken: 'access-token', + tokenType: '', + }, + updatedAt: Date.now(), + }, + expectedError: 'Token type is required', + }, + ])('should throw for $desc', ({ credentials, expectedError }) => { + expect(() => + storage.validateCredentials(credentials as OAuthCredentials), + ).toThrow(expectedError); }); }); describe('isTokenExpired', () => { - it('should return false for tokens without expiry', () => { + it.each([ + ['tokens without expiry', undefined, false], + ['valid tokens', Date.now() + 3600000, false], + ['expired tokens', Date.now() - 3600000, true], + [ + 'tokens within 5-minute buffer (4 minutes from now)', + Date.now() + 4 * 60 * 1000, + true, + ], + ])('should return %s for %s', (_, expiresAt, expected) => { const credentials: OAuthCredentials = { serverName: 'test-server', token: { accessToken: 'access-token', tokenType: 'Bearer', + ...(expiresAt !== undefined && { expiresAt }), }, updatedAt: Date.now(), }; - expect(storage.isTokenExpired(credentials)).toBe(false); - }); - - it('should return false for valid tokens', () => { - const credentials: OAuthCredentials = { - serverName: 'test-server', - token: { - accessToken: 'access-token', - tokenType: 'Bearer', - expiresAt: Date.now() + 3600000, - }, - updatedAt: Date.now(), - }; - - expect(storage.isTokenExpired(credentials)).toBe(false); - }); - - it('should return true for expired tokens', () => { - const credentials: OAuthCredentials = { - serverName: 'test-server', - token: { - accessToken: 'access-token', - tokenType: 'Bearer', - expiresAt: Date.now() - 3600000, - }, - updatedAt: Date.now(), - }; - - expect(storage.isTokenExpired(credentials)).toBe(true); - }); - - it('should apply 5-minute buffer for expiry check', () => { - const fourMinutesFromNow = Date.now() + 4 * 60 * 1000; - const credentials: OAuthCredentials = { - serverName: 'test-server', - token: { - accessToken: 'access-token', - tokenType: 'Bearer', - expiresAt: fourMinutesFromNow, - }, - updatedAt: Date.now(), - }; - - expect(storage.isTokenExpired(credentials)).toBe(true); + expect(storage.isTokenExpired(credentials)).toBe(expected); }); }); describe('sanitizeServerName', () => { - it('should keep valid characters', () => { - expect(storage.sanitizeServerName('test-server.example_123')).toBe( + it.each([ + [ + 'valid characters', 'test-server.example_123', - ); - }); - - it('should replace invalid characters with underscore', () => { - expect(storage.sanitizeServerName('test@server#example')).toBe( + 'test-server.example_123', + ], + [ + 'invalid characters with underscore replacement', + 'test@server#example', 'test_server_example', - ); - }); - - it('should handle special characters', () => { - expect(storage.sanitizeServerName('test server/example:123')).toBe( + ], + [ + 'special characters', + 'test server/example:123', 'test_server_example_123', - ); + ], + ])('should handle %s', (_, input, expected) => { + expect(storage.sanitizeServerName(input)).toBe(expected); }); }); }); From b77381750cdc4321851d6f0123025978fa8abfde Mon Sep 17 00:00:00 2001 From: joshualitt Date: Thu, 23 Oct 2025 15:01:02 -0700 Subject: [PATCH 09/12] feat(core) Bump get-ripgrep version. (#11698) --- package-lock.json | 9 +++++---- packages/core/package.json | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index dee492eb85..f30a484e63 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1781,9 +1781,10 @@ } }, "node_modules/@joshua.litt/get-ripgrep": { - "version": "0.0.2", - "resolved": "https://registry.npmjs.org/@joshua.litt/get-ripgrep/-/get-ripgrep-0.0.2.tgz", - "integrity": "sha512-cSHA+H+HEkOXeiCxrNvGj/pgv2Y0bfp4GbH3R87zr7Vob2pDUZV3BkUL9ucHMoDFID4GteSy5z5niN/lF9QeuQ==", + "version": "0.0.3", + "resolved": "https://registry.npmjs.org/@joshua.litt/get-ripgrep/-/get-ripgrep-0.0.3.tgz", + "integrity": "sha512-rycdieAKKqXi2bsM7G2ayDiNk5CAX8ZOzsTQsirfOqUKPef04Xw40BWGGyimaOOuvPgLWYt3tPnLLG3TvPXi5Q==", + "license": "MIT", "dependencies": { "@lvce-editor/verror": "^1.6.0", "execa": "^9.5.2", @@ -18103,7 +18104,7 @@ "@google-cloud/opentelemetry-cloud-monitoring-exporter": "^0.21.0", "@google-cloud/opentelemetry-cloud-trace-exporter": "^3.0.0", "@google/genai": "1.16.0", - "@joshua.litt/get-ripgrep": "^0.0.2", + "@joshua.litt/get-ripgrep": "^0.0.3", "@modelcontextprotocol/sdk": "^1.11.0", "@opentelemetry/api": "^1.9.0", "@opentelemetry/exporter-logs-otlp-grpc": "^0.203.0", diff --git a/packages/core/package.json b/packages/core/package.json index 312b41a099..10adf56555 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -24,7 +24,7 @@ "@google-cloud/opentelemetry-cloud-monitoring-exporter": "^0.21.0", "@google-cloud/opentelemetry-cloud-trace-exporter": "^3.0.0", "@google/genai": "1.16.0", - "@joshua.litt/get-ripgrep": "^0.0.2", + "@joshua.litt/get-ripgrep": "^0.0.3", "@modelcontextprotocol/sdk": "^1.11.0", "@opentelemetry/api": "^1.9.0", "@opentelemetry/exporter-logs-otlp-grpc": "^0.203.0", From b16fe7b646a75b57132a64568050c87ce7ba92a1 Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Thu, 23 Oct 2025 16:10:43 -0700 Subject: [PATCH 10/12] First take at mocking out gemini cli responses in integration tests (#11156) --- ...t-compress-interactive.compress-empty.json | 18 ++ ...compress-interactive.compress-failure.json | 40 ++++ ...context-compress-interactive.compress.json | 40 ++++ .../context-compress-interactive.test.ts | 77 +++++-- integration-tests/test-helper.ts | 13 +- packages/cli/src/config/config.ts | 6 + packages/cli/src/gemini.test.tsx | 1 + packages/core/src/config/config.ts | 3 + .../core/src/core/contentGenerator.test.ts | 23 ++ packages/core/src/core/contentGenerator.ts | 5 + .../src/core/fakeContentGenerator.test.ts | 205 ++++++++++++++++++ .../core/src/core/fakeContentGenerator.ts | 101 +++++++++ 12 files changed, 507 insertions(+), 25 deletions(-) create mode 100644 integration-tests/context-compress-interactive.compress-empty.json create mode 100644 integration-tests/context-compress-interactive.compress-failure.json create mode 100644 integration-tests/context-compress-interactive.compress.json create mode 100644 packages/core/src/core/fakeContentGenerator.test.ts create mode 100644 packages/core/src/core/fakeContentGenerator.ts diff --git a/integration-tests/context-compress-interactive.compress-empty.json b/integration-tests/context-compress-interactive.compress-empty.json new file mode 100644 index 0000000000..5366bf317b --- /dev/null +++ b/integration-tests/context-compress-interactive.compress-empty.json @@ -0,0 +1,18 @@ +{ + "generateContent": [ + { + "candidates": [ + { + "content": { + "role": "model", + "parts": [ + { + "text": "This is more than the 5 tokens we return below which will trigger an error" + } + ] + } + } + ] + } + ] +} diff --git a/integration-tests/context-compress-interactive.compress-failure.json b/integration-tests/context-compress-interactive.compress-failure.json new file mode 100644 index 0000000000..939189366b --- /dev/null +++ b/integration-tests/context-compress-interactive.compress-failure.json @@ -0,0 +1,40 @@ +{ + "generateContent": [ + { + "candidates": [ + { + "content": { + "role": "model", + "parts": [ + { + "text": "This is more than the 5 tokens we return below which will trigger an error" + } + ] + } + } + ] + } + ], + "generateContentStream": [ + [ + { + "candidates": [ + { + "content": { + "role": "model", + "parts": [ + { + "text": "The initial response from the model" + } + ] + }, + "finishReason": "STOP" + } + ], + "usageMetadata": { + "promptTokenCount": 5 + } + } + ] + ] +} diff --git a/integration-tests/context-compress-interactive.compress.json b/integration-tests/context-compress-interactive.compress.json new file mode 100644 index 0000000000..b9d470fc9c --- /dev/null +++ b/integration-tests/context-compress-interactive.compress.json @@ -0,0 +1,40 @@ +{ + "generateContent": [ + { + "candidates": [ + { + "content": { + "role": "model", + "parts": [ + { + "text": "A summary of the conversation." + } + ] + } + } + ] + } + ], + "generateContentStream": [ + [ + { + "candidates": [ + { + "content": { + "role": "model", + "parts": [ + { + "text": "The initial response from the model" + } + ] + }, + "finishReason": "STOP" + } + ], + "usageMetadata": { + "promptTokenCount": 100000 + } + } + ] + ] +} diff --git a/integration-tests/context-compress-interactive.test.ts b/integration-tests/context-compress-interactive.test.ts index 030efe512e..5be9b73141 100644 --- a/integration-tests/context-compress-interactive.test.ts +++ b/integration-tests/context-compress-interactive.test.ts @@ -6,6 +6,7 @@ import { expect, describe, it, beforeEach, afterEach } from 'vitest'; import { TestRig } from './test-helper.js'; +import { join } from 'node:path'; describe('Interactive Mode', () => { let rig: TestRig; @@ -18,50 +19,78 @@ describe('Interactive Mode', () => { await rig.cleanup(); }); - // TODO(#11062): Make this test reliable by not using the actual Gemini model - // We could not rely on the following mechanisms that have already shown to be - // flakey: - // 1. Asking a prompt like "Output 1000 tokens and the inventor of the lightbulb" - // --> This was b/c the model occasionally did not output einstein and - // we are not able to trigger the compression piece - // 2. Asking it to out a specific output and waiting for that. - // --> The expect catches the input and thinks that is the output so the - // /compress gets called too early - it.skip('should trigger chat compression with /compress command', async () => { - rig.setup('interactive-compress-success'); + it('should trigger chat compression with /compress command', async () => { + await rig.setup('interactive-compress-test', { + fakeResponsesPath: join( + import.meta.dirname, + 'context-compress-interactive.compress.json', + ), + }); const run = await rig.runInteractive(); - // Generate a long context to make compression viable. - const longPrompt = - 'Write a 200 word story about a robot. The story MUST end with the following output: THE_END'; + await run.type('Initial prompt'); + await run.type('\r'); - await run.sendKeys(longPrompt); - await run.sendKeys('\r'); - - // Wait for the specific end marker. - await run.expectText('THE_END', 30000); + await run.expectText('The initial response from the model', 5000); await run.type('/compress'); - await run.sendKeys('\r'); + await run.type('\r'); const foundEvent = await rig.waitForTelemetryEvent( 'chat_compression', - 90000, + 5000, ); expect(foundEvent, 'chat_compression telemetry event was not found').toBe( true, ); + + await run.expectText('Chat history compressed', 5000); }); - it('should handle /compress command on empty history', async () => { - rig.setup('interactive-compress-empty'); + it('should handle compression failure on token inflation', async () => { + await rig.setup('interactive-compress-failure', { + fakeResponsesPath: join( + import.meta.dirname, + 'context-compress-interactive.compress-failure.json', + ), + }); const run = await rig.runInteractive(); + await run.type('Initial prompt'); + await run.type('\r'); + + await run.expectText('The initial response from the model', 25000); + await run.type('/compress'); await run.type('\r'); - await run.expectText('Nothing to compress.', 25000); + await run.expectText('compression was not beneficial', 5000); + + // Verify no telemetry event is logged for NOOP + const foundEvent = await rig.waitForTelemetryEvent( + 'chat_compression', + 5000, + ); + expect( + foundEvent, + 'chat_compression telemetry event should be found for failures', + ).toBe(true); + }); + + it('should handle /compress command on empty history', async () => { + rig.setup('interactive-compress-empty', { + fakeResponsesPath: join( + import.meta.dirname, + 'context-compress-interactive.compress-empty.json', + ), + }); + + const run = await rig.runInteractive(); + await run.type('/compress'); + await run.type('\r'); + + await run.expectText('Nothing to compress.', 5000); // Verify no telemetry event is logged for NOOP const foundEvent = await rig.waitForTelemetryEvent( diff --git a/integration-tests/test-helper.ts b/integration-tests/test-helper.ts index fbc965c0ab..d5a9026726 100644 --- a/integration-tests/test-helper.ts +++ b/integration-tests/test-helper.ts @@ -255,6 +255,7 @@ export class TestRig { testDir: string | null; testName?: string; _lastRunStdout?: string; + fakeResponsesPath?: string; constructor() { this.bundlePath = join(__dirname, '..', 'bundle/gemini.js'); @@ -263,12 +264,19 @@ export class TestRig { setup( testName: string, - options: { settings?: Record } = {}, + options: { + settings?: Record; + fakeResponsesPath?: string; + } = {}, ) { this.testName = testName; const sanitizedName = sanitizeTestName(testName); this.testDir = join(env['INTEGRATION_TEST_FILE_DIR']!, sanitizedName); mkdirSync(this.testDir, { recursive: true }); + if (options.fakeResponsesPath) { + this.fakeResponsesPath = join(this.testDir, 'fake-responses.json'); + fs.copyFileSync(options.fakeResponsesPath, this.fakeResponsesPath); + } // Create a settings file to point the CLI to the local collector const geminiDir = join(this.testDir, GEMINI_DIR); @@ -335,6 +343,9 @@ export class TestRig { const initialArgs = isNpmReleaseTest ? extraInitialArgs : [this.bundlePath, ...extraInitialArgs]; + if (this.fakeResponsesPath) { + initialArgs.push('--fake-responses', this.fakeResponsesPath); + } return { command, initialArgs }; } diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 96ab5c04d1..f6ae37a0b6 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -68,6 +68,7 @@ export interface CliArgs { useSmartEdit: boolean | undefined; useWriteTodos: boolean | undefined; outputFormat: string | undefined; + fakeResponses: string | undefined; } export async function parseArguments(settings: Settings): Promise { @@ -193,6 +194,10 @@ export async function parseArguments(settings: Settings): Promise { description: 'The format of the CLI output.', choices: ['text', 'json', 'stream-json'], }) + .option('fake-responses', { + type: 'string', + description: 'Path to a file with fake model responses for testing.', + }) .deprecateOption( 'prompt', 'Use the positional prompt instead. This flag will be removed in a future version.', @@ -649,6 +654,7 @@ export async function loadCliConfig( settings.tools?.enableMessageBusIntegration ?? false, codebaseInvestigatorSettings: settings.experimental?.codebaseInvestigatorSettings, + fakeResponses: argv.fakeResponses, retryFetchErrors: settings.general?.retryFetchErrors ?? false, ptyInfo: ptyInfo?.name, }); diff --git a/packages/cli/src/gemini.test.tsx b/packages/cli/src/gemini.test.tsx index 931e35a3b5..e1c04e2cfd 100644 --- a/packages/cli/src/gemini.test.tsx +++ b/packages/cli/src/gemini.test.tsx @@ -339,6 +339,7 @@ describe('gemini.tsx main function kitty protocol', () => { useSmartEdit: undefined, useWriteTodos: undefined, outputFormat: undefined, + fakeResponses: undefined, }); await main(); diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index b7d1fa7add..78632d0480 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -283,6 +283,7 @@ export interface ConfigParameters { continueOnFailedApiCall?: boolean; retryFetchErrors?: boolean; enableShellOutputEfficiency?: boolean; + fakeResponses?: string; ptyInfo?: string; disableYoloMode?: boolean; } @@ -381,6 +382,7 @@ export class Config { private readonly continueOnFailedApiCall: boolean; private readonly retryFetchErrors: boolean; private readonly enableShellOutputEfficiency: boolean; + readonly fakeResponses?: string; private readonly disableYoloMode: boolean; constructor(params: ConfigParameters) { @@ -489,6 +491,7 @@ export class Config { params.enableShellOutputEfficiency ?? true; this.extensionManagement = params.extensionManagement ?? true; this.storage = new Storage(this.targetDir); + this.fakeResponses = params.fakeResponses; this.enablePromptCompletion = params.enablePromptCompletion ?? false; this.fileExclusions = new FileExclusions(this); this.eventEmitter = params.eventEmitter; diff --git a/packages/core/src/core/contentGenerator.test.ts b/packages/core/src/core/contentGenerator.test.ts index 3084c84bd4..e1431b3550 100644 --- a/packages/core/src/core/contentGenerator.test.ts +++ b/packages/core/src/core/contentGenerator.test.ts @@ -15,13 +15,36 @@ import { createCodeAssistContentGenerator } from '../code_assist/codeAssist.js'; import { GoogleGenAI } from '@google/genai'; import type { Config } from '../config/config.js'; import { LoggingContentGenerator } from './loggingContentGenerator.js'; +import { FakeContentGenerator } from './fakeContentGenerator.js'; vi.mock('../code_assist/codeAssist.js'); vi.mock('@google/genai'); +vi.mock('./fakeContentGenerator.js'); const mockConfig = {} as unknown as Config; describe('createContentGenerator', () => { + it('should create a FakeContentGenerator', async () => { + const mockGenerator = {} as unknown as ContentGenerator; + vi.mocked(FakeContentGenerator.fromFile).mockResolvedValue( + mockGenerator as never, + ); + const fakeResponsesFile = 'fake/responses.yaml'; + const mockConfigWithFake = { + fakeResponses: fakeResponsesFile, + } as unknown as Config; + const generator = await createContentGenerator( + { + authType: AuthType.USE_GEMINI, + }, + mockConfigWithFake, + ); + expect(FakeContentGenerator.fromFile).toHaveBeenCalledWith( + fakeResponsesFile, + ); + expect(generator).toEqual(mockGenerator); + }); + it('should create a CodeAssistContentGenerator', async () => { const mockGenerator = {} as unknown as ContentGenerator; vi.mocked(createCodeAssistContentGenerator).mockResolvedValue( diff --git a/packages/core/src/core/contentGenerator.ts b/packages/core/src/core/contentGenerator.ts index e4e8ebb861..487356a19e 100644 --- a/packages/core/src/core/contentGenerator.ts +++ b/packages/core/src/core/contentGenerator.ts @@ -19,6 +19,7 @@ import type { Config } from '../config/config.js'; import type { UserTierId } from '../code_assist/types.js'; import { LoggingContentGenerator } from './loggingContentGenerator.js'; import { InstallationManager } from '../utils/installationManager.js'; +import { FakeContentGenerator } from './fakeContentGenerator.js'; /** * Interface abstracting the core functionalities for generating content and counting tokens. @@ -105,6 +106,10 @@ export async function createContentGenerator( gcConfig: Config, sessionId?: string, ): Promise { + if (gcConfig.fakeResponses) { + return FakeContentGenerator.fromFile(gcConfig.fakeResponses); + } + const version = process.env['CLI_VERSION'] || process.version; const userAgent = `GeminiCLI/${version} (${process.platform}; ${process.arch})`; const baseHeaders: Record = { diff --git a/packages/core/src/core/fakeContentGenerator.test.ts b/packages/core/src/core/fakeContentGenerator.test.ts new file mode 100644 index 0000000000..5ccd92d5e3 --- /dev/null +++ b/packages/core/src/core/fakeContentGenerator.test.ts @@ -0,0 +1,205 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { FakeContentGenerator } from './fakeContentGenerator.js'; +import { promises } from 'node:fs'; +import type { FakeResponses } from './fakeContentGenerator.js'; +import type { + GenerateContentResponse, + CountTokensResponse, + EmbedContentResponse, + GenerateContentParameters, + CountTokensParameters, + EmbedContentParameters, +} from '@google/genai'; + +vi.mock('node:fs', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + promises: { + ...actual.promises, + readFile: vi.fn(), + }, + }; +}); + +const mockReadFile = vi.mocked(promises.readFile); + +describe('FakeContentGenerator', () => { + const fakeResponses: FakeResponses = { + generateContent: [ + { + candidates: [ + { content: { parts: [{ text: 'response1' }], role: 'model' } }, + ], + }, + ] as GenerateContentResponse[], + generateContentStream: [ + [ + { + candidates: [ + { content: { parts: [{ text: 'chunk1' }], role: 'model' } }, + ], + }, + { + candidates: [ + { content: { parts: [{ text: 'chunk2' }], role: 'model' } }, + ], + }, + ], + ] as GenerateContentResponse[][], + countTokens: [{ totalTokens: 10 }] as CountTokensResponse[], + embedContent: [ + { embeddings: [{ values: [1, 2, 3] }] }, + ] as EmbedContentResponse[], + }; + + beforeEach(() => { + vi.resetAllMocks(); + }); + + it('should return responses for generateContent', async () => { + const generator = new FakeContentGenerator(fakeResponses); + const response = await generator.generateContent( + {} as GenerateContentParameters, + 'id', + ); + expect(response).toEqual(fakeResponses.generateContent[0]); + }); + + it('should throw error when no more generateContent responses', async () => { + const generator = new FakeContentGenerator({ + ...fakeResponses, + generateContent: [], + }); + await expect( + generator.generateContent({} as GenerateContentParameters, 'id'), + ).rejects.toThrowError('No more mock responses for generateContent'); + }); + + it('should return responses for generateContentStream', async () => { + const generator = new FakeContentGenerator(fakeResponses); + const stream = await generator.generateContentStream( + {} as GenerateContentParameters, + 'id', + ); + const responses = []; + for await (const response of stream) { + responses.push(response); + } + expect(responses).toEqual(fakeResponses.generateContentStream[0]); + }); + + it('should throw error when no more generateContentStream responses', async () => { + const generator = new FakeContentGenerator({ + ...fakeResponses, + generateContentStream: [], + }); + await expect( + generator.generateContentStream({} as GenerateContentParameters, 'id'), + ).rejects.toThrow('No more mock responses for generateContentStream'); + }); + + it('should return responses for countTokens', async () => { + const generator = new FakeContentGenerator(fakeResponses); + const response = await generator.countTokens({} as CountTokensParameters); + expect(response).toEqual(fakeResponses.countTokens[0]); + }); + + it('should throw error when no more countTokens responses', async () => { + const generator = new FakeContentGenerator({ + ...fakeResponses, + countTokens: [], + }); + await expect( + generator.countTokens({} as CountTokensParameters), + ).rejects.toThrowError('No more mock responses for countTokens'); + }); + + it('should return responses for embedContent', async () => { + const generator = new FakeContentGenerator(fakeResponses); + const response = await generator.embedContent({} as EmbedContentParameters); + expect(response).toEqual(fakeResponses.embedContent[0]); + }); + + it('should throw error when no more embedContent responses', async () => { + const generator = new FakeContentGenerator({ + ...fakeResponses, + embedContent: [], + }); + await expect( + generator.embedContent({} as EmbedContentParameters), + ).rejects.toThrowError('No more mock responses for embedContent'); + }); + + it('should handle multiple calls and exhaust responses', async () => { + const generator = new FakeContentGenerator(fakeResponses); + await generator.generateContent({} as GenerateContentParameters, 'id'); + await expect( + generator.generateContent({} as GenerateContentParameters, 'id'), + ).rejects.toThrow(); + }); + + describe('fromFile', () => { + it('should create a generator from a file', async () => { + const fileContent = JSON.stringify(fakeResponses); + mockReadFile.mockResolvedValue(fileContent); + + const generator = await FakeContentGenerator.fromFile('fake-path.json'); + const response = await generator.generateContent( + {} as GenerateContentParameters, + 'id', + ); + expect(response).toEqual(fakeResponses.generateContent[0]); + }); + }); + + describe('constructor with partial responses', () => { + it('should handle missing generateContent', async () => { + const responses = { ...fakeResponses, generateContent: undefined }; + const generator = new FakeContentGenerator( + responses as unknown as FakeResponses, + ); + await expect( + generator.generateContent({} as GenerateContentParameters, 'id'), + ).rejects.toThrowError('No more mock responses for generateContent'); + }); + + it('should handle missing generateContentStream', async () => { + const responses = { ...fakeResponses, generateContentStream: undefined }; + const generator = new FakeContentGenerator( + responses as unknown as FakeResponses, + ); + await expect( + generator.generateContentStream({} as GenerateContentParameters, 'id'), + ).rejects.toThrowError( + 'No more mock responses for generateContentStream', + ); + }); + + it('should handle missing countTokens', async () => { + const responses = { ...fakeResponses, countTokens: undefined }; + const generator = new FakeContentGenerator( + responses as unknown as FakeResponses, + ); + await expect( + generator.countTokens({} as CountTokensParameters), + ).rejects.toThrowError('No more mock responses for countTokens'); + }); + + it('should handle missing embedContent', async () => { + const responses = { ...fakeResponses, embedContent: undefined }; + const generator = new FakeContentGenerator( + responses as unknown as FakeResponses, + ); + await expect( + generator.embedContent({} as EmbedContentParameters), + ).rejects.toThrowError('No more mock responses for embedContent'); + }); + }); +}); diff --git a/packages/core/src/core/fakeContentGenerator.ts b/packages/core/src/core/fakeContentGenerator.ts new file mode 100644 index 0000000000..9ef48b27e7 --- /dev/null +++ b/packages/core/src/core/fakeContentGenerator.ts @@ -0,0 +1,101 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { + CountTokensResponse, + GenerateContentResponse, + GenerateContentParameters, + CountTokensParameters, + EmbedContentResponse, + EmbedContentParameters, +} from '@google/genai'; +import { promises } from 'node:fs'; +import type { ContentGenerator } from './contentGenerator.js'; +import type { UserTierId } from '../code_assist/types.js'; +import { safeJsonStringify } from '../utils/safeJsonStringify.js'; + +export type FakeResponses = { + generateContent: GenerateContentResponse[]; + generateContentStream: GenerateContentResponse[][]; + countTokens: CountTokensResponse[]; + embedContent: EmbedContentResponse[]; +}; + +// A ContentGenerator that responds with canned responses. +// +// Typically these would come from a file, provided by the `--fake-responses` +// CLI argument. +export class FakeContentGenerator implements ContentGenerator { + private responses: FakeResponses; + private callCounters = { + generateContent: 0, + generateContentStream: 0, + countTokens: 0, + embedContent: 0, + }; + userTier?: UserTierId; + + constructor(responses: FakeResponses) { + this.responses = { + generateContent: responses.generateContent ?? [], + generateContentStream: responses.generateContentStream ?? [], + countTokens: responses.countTokens ?? [], + embedContent: responses.embedContent ?? [], + }; + } + + static async fromFile(filePath: string): Promise { + const fileContent = await promises.readFile(filePath, 'utf-8'); + const responses = JSON.parse(fileContent) as FakeResponses; + return new FakeContentGenerator(responses); + } + + private getNextResponse( + method: K, + request: unknown, + ): FakeResponses[K][number] { + const response = this.responses[method][this.callCounters[method]++]; + if (!response) { + throw new Error( + `No more mock responses for ${method}, got request:\n` + + safeJsonStringify(request), + ); + } + return response; + } + + async generateContent( + _request: GenerateContentParameters, + _userPromptId: string, + ): Promise { + return this.getNextResponse('generateContent', _request); + } + + async generateContentStream( + _request: GenerateContentParameters, + _userPromptId: string, + ): Promise> { + const responses = this.getNextResponse('generateContentStream', _request); + async function* stream() { + for (const response of responses) { + yield response; + } + } + return stream(); + } + + async countTokens( + _request: CountTokensParameters, + ): Promise { + return this.getNextResponse('countTokens', _request); + } + + async embedContent( + _request: EmbedContentParameters, + ): Promise { + return this.getNextResponse('embedContent', _request); + } +} From 0fe82a2f4e624fd70229be49da4a501f2f401d84 Mon Sep 17 00:00:00 2001 From: Tommaso Sciortino Date: Thu, 23 Oct 2025 16:55:30 -0700 Subject: [PATCH 11/12] Use raw writes to stdin in test (#11871) --- .../src/ui/contexts/KeypressContext.test.tsx | 201 +++++++----------- 1 file changed, 81 insertions(+), 120 deletions(-) diff --git a/packages/cli/src/ui/contexts/KeypressContext.test.tsx b/packages/cli/src/ui/contexts/KeypressContext.test.tsx index 719d938c62..295938ca9f 100644 --- a/packages/cli/src/ui/contexts/KeypressContext.test.tsx +++ b/packages/cli/src/ui/contexts/KeypressContext.test.tsx @@ -33,6 +33,9 @@ vi.mock('ink', async (importOriginal) => { const PASTE_START = '\x1B[200~'; const PASTE_END = '\x1B[201~'; +// readline will not emit most incomplete kitty sequences but it will give +// up on sequences like this where the modifier (135) has more than two digits. +const INCOMPLETE_KITTY_SEQUENCE = '\x1b[97;135'; class MockStdin extends EventEmitter { isTTY = true; @@ -45,15 +48,6 @@ class MockStdin extends EventEmitter { write(text: string) { this.emit('data', Buffer.from(text)); } - - /** - * Used to directly simulate keyPress events. Certain keypress events might - * be impossible to fire in certain versions of node. This allows us to - * sidestep readline entirely and just emit the keypress we want. - */ - pressKey(key: Partial) { - this.emit('keypress', null, key); - } } describe('KeypressContext - Kitty Protocol', () => { @@ -171,9 +165,7 @@ describe('KeypressContext - Kitty Protocol', () => { act(() => result.current.subscribe(keyHandler)); // Send kitty protocol sequence for numpad enter with Ctrl (modifier 5): ESC[57414;5u - act(() => { - stdin.write(`\x1b[57414;5u`); - }); + act(() => stdin.write(`\x1b[57414;5u`)); expect(keyHandler).toHaveBeenCalledWith( expect.objectContaining({ @@ -506,15 +498,14 @@ describe('KeypressContext - Kitty Protocol', () => { act(() => result.current.subscribe(keyHandler)); - // Send incomplete kitty sequence - act(() => stdin.pressKey({ sequence: '\x1b[1' })); + act(() => stdin.write(INCOMPLETE_KITTY_SEQUENCE)); // Send Ctrl+C act(() => stdin.write('\x03')); expect(consoleLogSpy).toHaveBeenCalledWith( '[DEBUG] Kitty buffer cleared on Ctrl+C:', - '\x1b[1', + INCOMPLETE_KITTY_SEQUENCE, ); // Verify Ctrl+C was handled @@ -543,19 +534,18 @@ describe('KeypressContext - Kitty Protocol', () => { act(() => result.current.subscribe(keyHandler)); // Send incomplete kitty sequence - const sequence = '\x1b[12'; - act(() => stdin.pressKey({ sequence })); + act(() => stdin.write(INCOMPLETE_KITTY_SEQUENCE)); // Verify debug logging for accumulation expect(consoleLogSpy).toHaveBeenCalledWith( '[DEBUG] Kitty buffer accumulating:', - JSON.stringify(sequence), + JSON.stringify(INCOMPLETE_KITTY_SEQUENCE), ); // Verify warning for char codes expect(consoleWarnSpy).toHaveBeenCalledWith( 'Kitty sequence buffer has content:', - JSON.stringify(sequence), + JSON.stringify(INCOMPLETE_KITTY_SEQUENCE), ); }); }); @@ -829,93 +819,75 @@ describe('Kitty Sequence Parsing', () => { // Terminals to test const terminals = ['iTerm2', 'Ghostty', 'MacTerminal', 'VSCodeTerminal']; - // Key mappings: letter -> [keycode, accented character, shouldHaveMeta] - // Note: µ (mu) is sent with meta:false on iTerm2/VSCode - const keys: Record = { - a: [97, 'å', true], - o: [111, 'ø', true], - m: [109, 'µ', false], + // Key mappings: letter -> [keycode, accented character] + const keys: Record = { + a: [97, 'å'], + o: [111, 'ø'], + m: [109, 'µ'], }; it.each( terminals.flatMap((terminal) => - Object.entries(keys).map( - ([key, [keycode, accentedChar, shouldHaveMeta]]) => { - if (terminal === 'Ghostty') { - // Ghostty uses kitty protocol sequences - return { - terminal, - key, - kittySequence: `\x1b[${keycode};3u`, - expected: { - name: key, - ctrl: false, - meta: true, - shift: false, - paste: false, - kittyProtocol: true, - }, - }; - } else if (terminal === 'MacTerminal') { - // Mac Terminal sends ESC + letter - return { - terminal, - key, - kitty: false, - input: { - sequence: `\x1b${key}`, - name: key, - ctrl: false, - meta: true, - shift: false, - paste: false, - }, - expected: { - sequence: `\x1b${key}`, - name: key, - ctrl: false, - meta: true, - shift: false, - paste: false, - }, - }; - } else { - // iTerm2 and VSCode send accented characters (å, ø, µ) - // Note: µ comes with meta:false but gets converted to m with meta:true - return { - terminal, - key, - input: { - name: key, - ctrl: false, - meta: shouldHaveMeta, - shift: false, - paste: false, - sequence: accentedChar, - }, - expected: { - name: key, - ctrl: false, - meta: true, // Always expect meta:true after conversion - shift: false, - paste: false, - sequence: accentedChar, - }, - }; - } - }, - ), + Object.entries(keys).map(([key, [keycode, accentedChar]]) => { + if (terminal === 'Ghostty') { + // Ghostty uses kitty protocol sequences + return { + terminal, + key, + chunk: `\x1b[${keycode};3u`, + expected: { + name: key, + ctrl: false, + meta: true, + shift: false, + paste: false, + kittyProtocol: true, + }, + }; + } else if (terminal === 'MacTerminal') { + // Mac Terminal sends ESC + letter + return { + terminal, + key, + kitty: false, + chunk: `\x1b${key}`, + expected: { + sequence: `\x1b${key}`, + name: key, + ctrl: false, + meta: true, + shift: false, + paste: false, + }, + }; + } else { + // iTerm2 and VSCode send accented characters (å, ø, µ) + // Note: µ (mu) is sent with meta:false on iTerm2/VSCode but + // gets converted to m with meta:true + return { + terminal, + key, + chunk: accentedChar, + expected: { + name: key, + ctrl: false, + meta: true, // Always expect meta:true after conversion + shift: false, + paste: false, + sequence: accentedChar, + }, + }; + } + }), ), )( 'should handle Alt+$key in $terminal', ({ - kittySequence, - input, + chunk, expected, kitty = true, }: { - kittySequence?: string; - input?: Partial; + chunk: string; expected: Partial; kitty?: boolean; }) => { @@ -930,11 +902,7 @@ describe('Kitty Sequence Parsing', () => { }); act(() => result.current.subscribe(keyHandler)); - if (kittySequence) { - act(() => stdin.write(kittySequence)); - } else if (input) { - act(() => stdin.pressKey(input)); - } + act(() => stdin.write(chunk)); expect(keyHandler).toHaveBeenCalledWith( expect.objectContaining(expected), @@ -978,8 +946,7 @@ describe('Kitty Sequence Parsing', () => { act(() => result.current.subscribe(keyHandler)); - // Send incomplete kitty sequence - act(() => stdin.pressKey({ sequence: '\x1b[1;' })); + act(() => stdin.write(INCOMPLETE_KITTY_SEQUENCE)); // Should not broadcast immediately expect(keyHandler).not.toHaveBeenCalled(); @@ -991,15 +958,13 @@ describe('Kitty Sequence Parsing', () => { expect(keyHandler).not.toHaveBeenCalled(); // Advance past timeout - act(() => { - vi.advanceTimersByTime(10); - }); + act(() => vi.advanceTimersByTime(10)); // Should now broadcast the incomplete sequence as regular input expect(keyHandler).toHaveBeenCalledWith( expect.objectContaining({ name: '', - sequence: '\x1b[1;', + sequence: INCOMPLETE_KITTY_SEQUENCE, paste: false, }), ); @@ -1079,8 +1044,7 @@ describe('Kitty Sequence Parsing', () => { act(() => result.current.subscribe(keyHandler)); - // Send incomplete kitty sequence - act(() => stdin.pressKey({ sequence: '\x1b[1;' })); + act(() => stdin.write(INCOMPLETE_KITTY_SEQUENCE)); // Press Ctrl+C act(() => stdin.write('\x03')); @@ -1189,13 +1153,13 @@ describe('Kitty Sequence Parsing', () => { act(() => result.current.subscribe(keyHandler)); // Start incomplete sequence - act(() => stdin.pressKey({ sequence: '\x1b[1' })); + act(() => stdin.write('\x1b[97;13')); // Advance time partway act(() => vi.advanceTimersByTime(30)); // Add more to sequence - act(() => stdin.write('3')); + act(() => stdin.write('5')); // Advance time from the first timeout point act(() => vi.advanceTimersByTime(25)); @@ -1209,7 +1173,7 @@ describe('Kitty Sequence Parsing', () => { // Should now parse as complete enter key expect(keyHandler).toHaveBeenCalledWith( expect.objectContaining({ - name: 'return', + name: 'a', kittyProtocol: true, }), ); @@ -1221,8 +1185,7 @@ describe('Kitty Sequence Parsing', () => { act(() => result.current.subscribe(keyHandler)); - // Send incomplete kitty sequence - act(() => stdin.pressKey({ sequence: '\x1b[1;' })); + act(() => stdin.write(INCOMPLETE_KITTY_SEQUENCE)); // Incomplete sequence should be buffered, not broadcast expect(keyHandler).not.toHaveBeenCalled(); @@ -1235,7 +1198,7 @@ describe('Kitty Sequence Parsing', () => { expect(keyHandler).toHaveBeenCalledWith( expect.objectContaining({ name: '', - sequence: '\x1b[1;', + sequence: INCOMPLETE_KITTY_SEQUENCE, paste: false, }), ); @@ -1247,8 +1210,7 @@ describe('Kitty Sequence Parsing', () => { act(() => result.current.subscribe(keyHandler)); - // Send incomplete kitty sequence - act(() => stdin.pressKey({ sequence: '\x1b[1;' })); + act(() => stdin.write(INCOMPLETE_KITTY_SEQUENCE)); // Incomplete sequence should be buffered, not broadcast expect(keyHandler).not.toHaveBeenCalled(); @@ -1261,7 +1223,7 @@ describe('Kitty Sequence Parsing', () => { expect(keyHandler).toHaveBeenCalledWith( expect.objectContaining({ name: '', - sequence: '\x1b[1;', + sequence: INCOMPLETE_KITTY_SEQUENCE, paste: false, }), ); @@ -1274,8 +1236,7 @@ describe('Kitty Sequence Parsing', () => { act(() => result.current.subscribe(keyHandler)); - // Send incomplete kitty sequence - act(() => stdin.pressKey({ sequence: '\x1b[1;' })); + act(() => stdin.write(INCOMPLETE_KITTY_SEQUENCE)); // Incomplete sequence should be buffered, not broadcast expect(keyHandler).not.toHaveBeenCalled(); @@ -1288,7 +1249,7 @@ describe('Kitty Sequence Parsing', () => { expect(keyHandler).toHaveBeenCalledWith( expect.objectContaining({ name: '', - sequence: '\x1b[1;', + sequence: INCOMPLETE_KITTY_SEQUENCE, paste: false, }), ); From 884d838a1e0e41e67c8614fdb7f6f2eddfe2066c Mon Sep 17 00:00:00 2001 From: Sandy Tao Date: Thu, 23 Oct 2025 18:52:16 -0700 Subject: [PATCH 12/12] fix(cli): re-throw errors in non-interactive mode (#11849) --- packages/cli/src/nonInteractiveCli.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/nonInteractiveCli.ts b/packages/cli/src/nonInteractiveCli.ts index 2d8d845114..7b89732b10 100644 --- a/packages/cli/src/nonInteractiveCli.ts +++ b/packages/cli/src/nonInteractiveCli.ts @@ -71,6 +71,7 @@ export async function runNonInteractive( ? new StreamJsonFormatter() : null; + let errorToHandle: unknown | undefined; try { consolePatcher.patch(); coreEvents.on(CoreEvent.UserFeedback, handleUserFeedback); @@ -213,6 +214,8 @@ export async function runNonInteractive( message: 'Maximum session turns exceeded', }); } + } else if (event.type === GeminiEventType.Error) { + throw event.value.error; } } @@ -302,7 +305,7 @@ export async function runNonInteractive( } } } catch (error) { - handleError(error, config); + errorToHandle = error; } finally { consolePatcher.cleanup(); coreEvents.off(CoreEvent.UserFeedback, handleUserFeedback); @@ -310,5 +313,9 @@ export async function runNonInteractive( await shutdownTelemetry(config); } } + + if (errorToHandle) { + handleError(errorToHandle, config); + } }); }