From 52e340a11bc6c9ca10674799fa784566d4bbd538 Mon Sep 17 00:00:00 2001 From: Jacob Richman Date: Wed, 20 Aug 2025 12:49:15 -0700 Subject: Revert "Ignore workspace settings for untrusted folders" (#6672) --- packages/cli/src/ui/hooks/useAuthCommand.ts | 8 +- .../cli/src/ui/hooks/useEditorSettings.test.ts | 283 +++++++++++++++++++ .../cli/src/ui/hooks/useEditorSettings.test.tsx | 305 --------------------- packages/cli/src/ui/hooks/useEditorSettings.ts | 15 +- packages/cli/src/ui/hooks/useFolderTrust.ts | 7 +- packages/cli/src/ui/hooks/useThemeCommand.ts | 10 +- 6 files changed, 297 insertions(+), 331 deletions(-) create mode 100644 packages/cli/src/ui/hooks/useEditorSettings.test.ts delete mode 100644 packages/cli/src/ui/hooks/useEditorSettings.test.tsx (limited to 'packages/cli/src/ui/hooks') diff --git a/packages/cli/src/ui/hooks/useAuthCommand.ts b/packages/cli/src/ui/hooks/useAuthCommand.ts index b92fe604..e57a11af 100644 --- a/packages/cli/src/ui/hooks/useAuthCommand.ts +++ b/packages/cli/src/ui/hooks/useAuthCommand.ts @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { useState, useCallback, useEffect, useContext } from 'react'; +import { useState, useCallback, useEffect } from 'react'; import { LoadedSettings, SettingScope } from '../../config/settings.js'; import { AuthType, @@ -13,7 +13,6 @@ import { getErrorMessage, } from '@google/gemini-cli-core'; import { runExitCleanup } from '../../utils/cleanup.js'; -import { SettingsContext } from '../contexts/SettingsContext.js'; export const useAuthCommand = ( settings: LoadedSettings, @@ -23,7 +22,6 @@ export const useAuthCommand = ( const [isAuthDialogOpen, setIsAuthDialogOpen] = useState( settings.merged.selectedAuthType === undefined, ); - const settingsContext = useContext(SettingsContext); const openAuthDialog = useCallback(() => { setIsAuthDialogOpen(true); @@ -58,7 +56,7 @@ export const useAuthCommand = ( if (authType) { await clearCachedCredentialFile(); - settingsContext?.settings.setValue(scope, 'selectedAuthType', authType); + settings.setValue(scope, 'selectedAuthType', authType); if ( authType === AuthType.LOGIN_WITH_GOOGLE && config.isBrowserLaunchSuppressed() @@ -77,7 +75,7 @@ Logging in with Google... Please restart Gemini CLI to continue. setIsAuthDialogOpen(false); setAuthError(null); }, - [settingsContext, setAuthError, config], + [settings, setAuthError, config], ); const cancelAuthentication = useCallback(() => { diff --git a/packages/cli/src/ui/hooks/useEditorSettings.test.ts b/packages/cli/src/ui/hooks/useEditorSettings.test.ts new file mode 100644 index 00000000..7b056c2a --- /dev/null +++ b/packages/cli/src/ui/hooks/useEditorSettings.test.ts @@ -0,0 +1,283 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + afterEach, + beforeEach, + describe, + expect, + it, + vi, + type MockedFunction, +} from 'vitest'; +import { act } from 'react'; +import { renderHook } from '@testing-library/react'; +import { useEditorSettings } from './useEditorSettings.js'; +import { LoadedSettings, SettingScope } from '../../config/settings.js'; +import { MessageType, type HistoryItem } from '../types.js'; +import { + type EditorType, + checkHasEditorType, + allowEditorTypeInSandbox, +} from '@google/gemini-cli-core'; + +vi.mock('@google/gemini-cli-core', async () => { + const actual = await vi.importActual('@google/gemini-cli-core'); + return { + ...actual, + checkHasEditorType: vi.fn(() => true), + allowEditorTypeInSandbox: vi.fn(() => true), + }; +}); + +const mockCheckHasEditorType = vi.mocked(checkHasEditorType); +const mockAllowEditorTypeInSandbox = vi.mocked(allowEditorTypeInSandbox); + +describe('useEditorSettings', () => { + let mockLoadedSettings: LoadedSettings; + let mockSetEditorError: MockedFunction<(error: string | null) => void>; + let mockAddItem: MockedFunction< + (item: Omit, timestamp: number) => void + >; + + beforeEach(() => { + vi.resetAllMocks(); + + mockLoadedSettings = { + setValue: vi.fn(), + } as unknown as LoadedSettings; + + mockSetEditorError = vi.fn(); + mockAddItem = vi.fn(); + + // Reset mock implementations to default + mockCheckHasEditorType.mockReturnValue(true); + mockAllowEditorTypeInSandbox.mockReturnValue(true); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('should initialize with dialog closed', () => { + const { result } = renderHook(() => + useEditorSettings(mockLoadedSettings, mockSetEditorError, mockAddItem), + ); + + expect(result.current.isEditorDialogOpen).toBe(false); + }); + + it('should open editor dialog when openEditorDialog is called', () => { + const { result } = renderHook(() => + useEditorSettings(mockLoadedSettings, mockSetEditorError, mockAddItem), + ); + + act(() => { + result.current.openEditorDialog(); + }); + + expect(result.current.isEditorDialogOpen).toBe(true); + }); + + it('should close editor dialog when exitEditorDialog is called', () => { + const { result } = renderHook(() => + useEditorSettings(mockLoadedSettings, mockSetEditorError, mockAddItem), + ); + act(() => { + result.current.openEditorDialog(); + result.current.exitEditorDialog(); + }); + expect(result.current.isEditorDialogOpen).toBe(false); + }); + + it('should handle editor selection successfully', () => { + const { result } = renderHook(() => + useEditorSettings(mockLoadedSettings, mockSetEditorError, mockAddItem), + ); + + const editorType: EditorType = 'vscode'; + const scope = SettingScope.User; + + act(() => { + result.current.openEditorDialog(); + result.current.handleEditorSelect(editorType, scope); + }); + + expect(mockLoadedSettings.setValue).toHaveBeenCalledWith( + scope, + 'preferredEditor', + editorType, + ); + + expect(mockAddItem).toHaveBeenCalledWith( + { + type: MessageType.INFO, + text: 'Editor preference set to "vscode" in User settings.', + }, + expect.any(Number), + ); + + expect(mockSetEditorError).toHaveBeenCalledWith(null); + expect(result.current.isEditorDialogOpen).toBe(false); + }); + + it('should handle clearing editor preference (undefined editor)', () => { + const { result } = renderHook(() => + useEditorSettings(mockLoadedSettings, mockSetEditorError, mockAddItem), + ); + + const scope = SettingScope.Workspace; + + act(() => { + result.current.openEditorDialog(); + result.current.handleEditorSelect(undefined, scope); + }); + + expect(mockLoadedSettings.setValue).toHaveBeenCalledWith( + scope, + 'preferredEditor', + undefined, + ); + + expect(mockAddItem).toHaveBeenCalledWith( + { + type: MessageType.INFO, + text: 'Editor preference cleared in Workspace settings.', + }, + expect.any(Number), + ); + + expect(mockSetEditorError).toHaveBeenCalledWith(null); + expect(result.current.isEditorDialogOpen).toBe(false); + }); + + it('should handle different editor types', () => { + const { result } = renderHook(() => + useEditorSettings(mockLoadedSettings, mockSetEditorError, mockAddItem), + ); + + const editorTypes: EditorType[] = ['cursor', 'windsurf', 'vim']; + const scope = SettingScope.User; + + editorTypes.forEach((editorType) => { + act(() => { + result.current.handleEditorSelect(editorType, scope); + }); + + expect(mockLoadedSettings.setValue).toHaveBeenCalledWith( + scope, + 'preferredEditor', + editorType, + ); + + expect(mockAddItem).toHaveBeenCalledWith( + { + type: MessageType.INFO, + text: `Editor preference set to "${editorType}" in User settings.`, + }, + expect.any(Number), + ); + }); + }); + + it('should handle different setting scopes', () => { + const { result } = renderHook(() => + useEditorSettings(mockLoadedSettings, mockSetEditorError, mockAddItem), + ); + + const editorType: EditorType = 'vscode'; + const scopes = [SettingScope.User, SettingScope.Workspace]; + + scopes.forEach((scope) => { + act(() => { + result.current.handleEditorSelect(editorType, scope); + }); + + expect(mockLoadedSettings.setValue).toHaveBeenCalledWith( + scope, + 'preferredEditor', + editorType, + ); + + expect(mockAddItem).toHaveBeenCalledWith( + { + type: MessageType.INFO, + text: `Editor preference set to "vscode" in ${scope} settings.`, + }, + expect.any(Number), + ); + }); + }); + + it('should not set preference for unavailable editors', () => { + const { result } = renderHook(() => + useEditorSettings(mockLoadedSettings, mockSetEditorError, mockAddItem), + ); + + mockCheckHasEditorType.mockReturnValue(false); + + const editorType: EditorType = 'vscode'; + const scope = SettingScope.User; + + act(() => { + result.current.openEditorDialog(); + result.current.handleEditorSelect(editorType, scope); + }); + + expect(mockLoadedSettings.setValue).not.toHaveBeenCalled(); + expect(mockAddItem).not.toHaveBeenCalled(); + expect(result.current.isEditorDialogOpen).toBe(true); + }); + + it('should not set preference for editors not allowed in sandbox', () => { + const { result } = renderHook(() => + useEditorSettings(mockLoadedSettings, mockSetEditorError, mockAddItem), + ); + + mockAllowEditorTypeInSandbox.mockReturnValue(false); + + const editorType: EditorType = 'vscode'; + const scope = SettingScope.User; + + act(() => { + result.current.openEditorDialog(); + result.current.handleEditorSelect(editorType, scope); + }); + + expect(mockLoadedSettings.setValue).not.toHaveBeenCalled(); + expect(mockAddItem).not.toHaveBeenCalled(); + expect(result.current.isEditorDialogOpen).toBe(true); + }); + + it('should handle errors during editor selection', () => { + const { result } = renderHook(() => + useEditorSettings(mockLoadedSettings, mockSetEditorError, mockAddItem), + ); + + const errorMessage = 'Failed to save settings'; + ( + mockLoadedSettings.setValue as MockedFunction< + typeof mockLoadedSettings.setValue + > + ).mockImplementation(() => { + throw new Error(errorMessage); + }); + + const editorType: EditorType = 'vscode'; + const scope = SettingScope.User; + + act(() => { + result.current.openEditorDialog(); + result.current.handleEditorSelect(editorType, scope); + }); + + expect(mockSetEditorError).toHaveBeenCalledWith( + `Failed to set editor preference: Error: ${errorMessage}`, + ); + expect(mockAddItem).not.toHaveBeenCalled(); + expect(result.current.isEditorDialogOpen).toBe(true); + }); +}); diff --git a/packages/cli/src/ui/hooks/useEditorSettings.test.tsx b/packages/cli/src/ui/hooks/useEditorSettings.test.tsx deleted file mode 100644 index f1d65056..00000000 --- a/packages/cli/src/ui/hooks/useEditorSettings.test.tsx +++ /dev/null @@ -1,305 +0,0 @@ -/** - * @license - * Copyright 2025 Google LLC - * SPDX-License-Identifier: Apache-2.0 - */ - -import { - afterEach, - beforeEach, - describe, - expect, - it, - vi, - type MockedFunction, -} from 'vitest'; -import { act } from 'react'; -import { renderHook } from '@testing-library/react'; -import { useEditorSettings } from './useEditorSettings.js'; -import { LoadedSettings, SettingScope } from '../../config/settings.js'; -import { MessageType, type HistoryItem } from '../types.js'; -import { - type EditorType, - checkHasEditorType, - allowEditorTypeInSandbox, -} from '@google/gemini-cli-core'; -import { SettingsContext } from '../contexts/SettingsContext.js'; -import { type ReactNode } from 'react'; - -vi.mock('@google/gemini-cli-core', async () => { - const actual = await vi.importActual('@google/gemini-cli-core'); - return { - ...actual, - checkHasEditorType: vi.fn(() => true), - allowEditorTypeInSandbox: vi.fn(() => true), - }; -}); - -const mockCheckHasEditorType = vi.mocked(checkHasEditorType); -const mockAllowEditorTypeInSandbox = vi.mocked(allowEditorTypeInSandbox); - -describe('useEditorSettings', () => { - let mockLoadedSettings: LoadedSettings; - let mockSetEditorError: MockedFunction<(error: string | null) => void>; - let mockAddItem: MockedFunction< - (item: Omit, timestamp: number) => void - >; - - const wrapper = ({ children }: { children: ReactNode }) => ( - {} }} - > - {children} - - ); - - beforeEach(() => { - vi.resetAllMocks(); - mockLoadedSettings = new LoadedSettings( - { path: '', settings: {} }, - { path: '', settings: {} }, - { path: '', settings: {} }, - [], - ); - mockLoadedSettings.setValue = vi.fn(); - mockSetEditorError = vi.fn(); - mockAddItem = vi.fn(); - - // Reset mock implementations to default - mockCheckHasEditorType.mockReturnValue(true); - mockAllowEditorTypeInSandbox.mockReturnValue(true); - }); - - afterEach(() => { - vi.restoreAllMocks(); - }); - - it('should initialize with dialog closed', () => { - const { result } = renderHook( - () => useEditorSettings(mockSetEditorError, mockAddItem), - { wrapper }, - ); - - expect(result.current.isEditorDialogOpen).toBe(false); - }); - - it('should open editor dialog when openEditorDialog is called', () => { - const { result } = renderHook( - () => useEditorSettings(mockSetEditorError, mockAddItem), - { wrapper }, - ); - - act(() => { - result.current.openEditorDialog(); - }); - - expect(result.current.isEditorDialogOpen).toBe(true); - }); - - it('should close editor dialog when exitEditorDialog is called', () => { - const { result } = renderHook( - () => useEditorSettings(mockSetEditorError, mockAddItem), - { wrapper }, - ); - act(() => { - result.current.openEditorDialog(); - result.current.exitEditorDialog(); - }); - expect(result.current.isEditorDialogOpen).toBe(false); - }); - - it('should handle editor selection successfully', () => { - const { result } = renderHook( - () => useEditorSettings(mockSetEditorError, mockAddItem), - { wrapper }, - ); - - const editorType: EditorType = 'vscode'; - const scope = SettingScope.User; - - act(() => { - result.current.openEditorDialog(); - result.current.handleEditorSelect(editorType, scope); - }); - - expect(mockLoadedSettings.setValue).toHaveBeenCalledWith( - scope, - 'preferredEditor', - editorType, - ); - - expect(mockAddItem).toHaveBeenCalledWith( - { - type: MessageType.INFO, - text: 'Editor preference set to "vscode" in User settings.', - }, - expect.any(Number), - ); - - expect(mockSetEditorError).toHaveBeenCalledWith(null); - expect(result.current.isEditorDialogOpen).toBe(false); - }); - - it('should handle clearing editor preference (undefined editor)', () => { - const { result } = renderHook( - () => useEditorSettings(mockSetEditorError, mockAddItem), - { wrapper }, - ); - - const scope = SettingScope.Workspace; - - act(() => { - result.current.openEditorDialog(); - result.current.handleEditorSelect(undefined, scope); - }); - - expect(mockLoadedSettings.setValue).toHaveBeenCalledWith( - scope, - 'preferredEditor', - undefined, - ); - - expect(mockAddItem).toHaveBeenCalledWith( - { - type: MessageType.INFO, - text: 'Editor preference cleared in Workspace settings.', - }, - expect.any(Number), - ); - - expect(mockSetEditorError).toHaveBeenCalledWith(null); - expect(result.current.isEditorDialogOpen).toBe(false); - }); - - it('should handle different editor types', () => { - const { result } = renderHook( - () => useEditorSettings(mockSetEditorError, mockAddItem), - { wrapper }, - ); - - const editorTypes: EditorType[] = ['cursor', 'windsurf', 'vim']; - const scope = SettingScope.User; - - editorTypes.forEach((editorType) => { - act(() => { - result.current.handleEditorSelect(editorType, scope); - }); - - expect(mockLoadedSettings.setValue).toHaveBeenCalledWith( - scope, - 'preferredEditor', - editorType, - ); - - expect(mockAddItem).toHaveBeenCalledWith( - { - type: MessageType.INFO, - text: `Editor preference set to "${editorType}" in User settings.`, - }, - expect.any(Number), - ); - }); - }); - - it('should handle different setting scopes', () => { - const { result } = renderHook( - () => useEditorSettings(mockSetEditorError, mockAddItem), - { wrapper }, - ); - - const editorType: EditorType = 'vscode'; - const scopes = [SettingScope.User, SettingScope.Workspace]; - - scopes.forEach((scope) => { - act(() => { - result.current.handleEditorSelect(editorType, scope); - }); - - expect(mockLoadedSettings.setValue).toHaveBeenCalledWith( - scope, - 'preferredEditor', - editorType, - ); - - expect(mockAddItem).toHaveBeenCalledWith( - { - type: MessageType.INFO, - text: `Editor preference set to "vscode" in ${scope} settings.`, - }, - expect.any(Number), - ); - }); - }); - - it('should not set preference for unavailable editors', () => { - const { result } = renderHook( - () => useEditorSettings(mockSetEditorError, mockAddItem), - { wrapper }, - ); - - mockCheckHasEditorType.mockReturnValue(false); - - const editorType: EditorType = 'vscode'; - const scope = SettingScope.User; - - act(() => { - result.current.openEditorDialog(); - result.current.handleEditorSelect(editorType, scope); - }); - - expect(mockLoadedSettings.setValue).not.toHaveBeenCalled(); - expect(mockAddItem).not.toHaveBeenCalled(); - expect(result.current.isEditorDialogOpen).toBe(true); - }); - - it('should not set preference for editors not allowed in sandbox', () => { - const { result } = renderHook( - () => useEditorSettings(mockSetEditorError, mockAddItem), - { wrapper }, - ); - - mockAllowEditorTypeInSandbox.mockReturnValue(false); - - const editorType: EditorType = 'vscode'; - const scope = SettingScope.User; - - act(() => { - result.current.openEditorDialog(); - result.current.handleEditorSelect(editorType, scope); - }); - - expect(mockLoadedSettings.setValue).not.toHaveBeenCalled(); - expect(mockAddItem).not.toHaveBeenCalled(); - expect(result.current.isEditorDialogOpen).toBe(true); - }); - - it('should handle errors during editor selection', () => { - const { result } = renderHook( - () => useEditorSettings(mockSetEditorError, mockAddItem), - { wrapper }, - ); - - const errorMessage = 'Failed to save settings'; - ( - mockLoadedSettings.setValue as MockedFunction< - typeof mockLoadedSettings.setValue - > - ).mockImplementation(() => { - throw new Error(errorMessage); - }); - - const editorType: EditorType = 'vscode'; - const scope = SettingScope.User; - - act(() => { - result.current.openEditorDialog(); - result.current.handleEditorSelect(editorType, scope); - }); - - expect(mockSetEditorError).toHaveBeenCalledWith( - `Failed to set editor preference: Error: ${errorMessage}`, - ); - expect(mockAddItem).not.toHaveBeenCalled(); - expect(result.current.isEditorDialogOpen).toBe(true); - }); -}); diff --git a/packages/cli/src/ui/hooks/useEditorSettings.ts b/packages/cli/src/ui/hooks/useEditorSettings.ts index bd6f72bb..60c16798 100644 --- a/packages/cli/src/ui/hooks/useEditorSettings.ts +++ b/packages/cli/src/ui/hooks/useEditorSettings.ts @@ -4,15 +4,14 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { useState, useCallback, useContext } from 'react'; -import { SettingScope } from '../../config/settings.js'; +import { useState, useCallback } from 'react'; +import { LoadedSettings, SettingScope } from '../../config/settings.js'; import { type HistoryItem, MessageType } from '../types.js'; import { allowEditorTypeInSandbox, checkHasEditorType, EditorType, } from '@google/gemini-cli-core'; -import { SettingsContext } from '../contexts/SettingsContext.js'; interface UseEditorSettingsReturn { isEditorDialogOpen: boolean; @@ -25,11 +24,11 @@ interface UseEditorSettingsReturn { } export const useEditorSettings = ( + loadedSettings: LoadedSettings, setEditorError: (error: string | null) => void, addItem: (item: Omit, timestamp: number) => void, ): UseEditorSettingsReturn => { const [isEditorDialogOpen, setIsEditorDialogOpen] = useState(false); - const settingsContext = useContext(SettingsContext); const openEditorDialog = useCallback(() => { setIsEditorDialogOpen(true); @@ -46,11 +45,7 @@ export const useEditorSettings = ( } try { - settingsContext?.settings.setValue( - scope, - 'preferredEditor', - editorType, - ); + loadedSettings.setValue(scope, 'preferredEditor', editorType); addItem( { type: MessageType.INFO, @@ -64,7 +59,7 @@ export const useEditorSettings = ( setEditorError(`Failed to set editor preference: ${error}`); } }, - [settingsContext, setEditorError, addItem], + [loadedSettings, setEditorError, addItem], ); const exitEditorDialog = useCallback(() => { diff --git a/packages/cli/src/ui/hooks/useFolderTrust.ts b/packages/cli/src/ui/hooks/useFolderTrust.ts index 560a2260..28b82b30 100644 --- a/packages/cli/src/ui/hooks/useFolderTrust.ts +++ b/packages/cli/src/ui/hooks/useFolderTrust.ts @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { useState, useCallback, useEffect, useContext } from 'react'; +import { useState, useCallback, useEffect } from 'react'; import { Settings, LoadedSettings } from '../../config/settings.js'; import { FolderTrustChoice } from '../components/FolderTrustDialog.js'; import { @@ -13,7 +13,6 @@ import { isWorkspaceTrusted, } from '../../config/trustedFolders.js'; import * as process from 'process'; -import { SettingsContext } from '../contexts/SettingsContext.js'; export const useFolderTrust = ( settings: LoadedSettings, @@ -21,7 +20,6 @@ export const useFolderTrust = ( ) => { const [isTrusted, setIsTrusted] = useState(undefined); const [isFolderTrustDialogOpen, setIsFolderTrustDialogOpen] = useState(false); - const settingsContext = useContext(SettingsContext); const { folderTrust, folderTrustFeature } = settings.merged; useEffect(() => { @@ -62,9 +60,8 @@ export const useFolderTrust = ( setIsTrusted(trusted); setIsFolderTrustDialogOpen(false); onTrustChange(trusted); - settingsContext?.recomputeSettings(); }, - [onTrustChange, folderTrust, folderTrustFeature, settingsContext], + [onTrustChange, folderTrust, folderTrustFeature], ); return { diff --git a/packages/cli/src/ui/hooks/useThemeCommand.ts b/packages/cli/src/ui/hooks/useThemeCommand.ts index 06d1c5b1..cf881f53 100644 --- a/packages/cli/src/ui/hooks/useThemeCommand.ts +++ b/packages/cli/src/ui/hooks/useThemeCommand.ts @@ -4,11 +4,10 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { useState, useCallback, useEffect, useContext } from 'react'; +import { useState, useCallback, useEffect } from 'react'; import { themeManager } from '../themes/theme-manager.js'; -import { HistoryItem, MessageType } from '../types.js'; -import { SettingScope } from '../../config/settings.js'; -import { SettingsContext } from '../contexts/SettingsContext.js'; +import { LoadedSettings, SettingScope } from '../../config/settings.js'; // Import LoadedSettings, AppSettings, MergedSetting +import { type HistoryItem, MessageType } from '../types.js'; import process from 'node:process'; interface UseThemeCommandReturn { @@ -22,12 +21,11 @@ interface UseThemeCommandReturn { } export const useThemeCommand = ( + loadedSettings: LoadedSettings, setThemeError: (error: string | null) => void, addItem: (item: Omit, timestamp: number) => void, ): UseThemeCommandReturn => { const [isThemeDialogOpen, setIsThemeDialogOpen] = useState(false); - const settingsContext = useContext(SettingsContext); - const loadedSettings = settingsContext!.settings; // Check for invalid theme configuration on startup useEffect(() => { -- cgit v1.2.3