diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 1aee75940b..440f6e7a90 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -413,7 +413,7 @@ export async function loadCliConfig( const ideMode = settings.ide?.enabled ?? false; const folderTrust = settings.security?.folderTrust?.enabled ?? false; - const trustedFolder = isWorkspaceTrusted(settings)?.isTrusted ?? true; + const trustedFolder = isWorkspaceTrusted(settings)?.isTrusted ?? false; // Set the context filename in the server's memoryTool module BEFORE loading memory // TODO(b/343434939): This is a bit of a hack. The contextFileName should ideally be passed diff --git a/packages/cli/src/ui/components/FolderTrustDialog.test.tsx b/packages/cli/src/ui/components/FolderTrustDialog.test.tsx index 2ba6723a5f..7d881a72fb 100644 --- a/packages/cli/src/ui/components/FolderTrustDialog.test.tsx +++ b/packages/cli/src/ui/components/FolderTrustDialog.test.tsx @@ -7,7 +7,7 @@ import { renderWithProviders } from '../../test-utils/render.js'; import { waitFor } from '../../test-utils/async.js'; import { act } from 'react'; -import { vi } from 'vitest'; +import { vi, describe, it, expect, beforeEach } from 'vitest'; import { FolderTrustDialog } from './FolderTrustDialog.js'; import { ExitCodes } from '@google/gemini-cli-core'; import * as processUtils from '../../utils/processUtils.js'; @@ -74,7 +74,7 @@ describe('FolderTrustDialog', () => { , ); - expect(lastFrame()).toContain(' Gemini CLI is restarting'); + expect(lastFrame()).toContain('Gemini CLI is restarting'); }); it('should call relaunchApp when isRestarting is true', async () => { @@ -88,6 +88,21 @@ describe('FolderTrustDialog', () => { vi.useRealTimers(); }); + it('should not call relaunchApp if unmounted before timeout', async () => { + vi.useFakeTimers(); + const relaunchApp = vi.spyOn(processUtils, 'relaunchApp'); + const { unmount } = renderWithProviders( + , + ); + + // Unmount immediately (before 250ms) + unmount(); + + await vi.advanceTimersByTimeAsync(250); + expect(relaunchApp).not.toHaveBeenCalled(); + vi.useRealTimers(); + }); + it('should not call process.exit when "r" is pressed and isRestarting is false', async () => { const { stdin } = renderWithProviders( , diff --git a/packages/cli/src/ui/components/FolderTrustDialog.tsx b/packages/cli/src/ui/components/FolderTrustDialog.tsx index c5a5c9269f..b945739304 100644 --- a/packages/cli/src/ui/components/FolderTrustDialog.tsx +++ b/packages/cli/src/ui/components/FolderTrustDialog.tsx @@ -6,7 +6,7 @@ import { Box, Text } from 'ink'; import type React from 'react'; -import { useEffect, useState } from 'react'; +import { useEffect, useState, useCallback } from 'react'; import { theme } from '../semantic-colors.js'; import type { RadioSelectItem } from './shared/RadioButtonSelect.js'; import { RadioButtonSelect } from './shared/RadioButtonSelect.js'; @@ -33,26 +33,32 @@ export const FolderTrustDialog: React.FC = ({ isRestarting, }) => { const [exiting, setExiting] = useState(false); + useEffect(() => { - const doRelaunch = async () => { - if (isRestarting) { - setTimeout(async () => { - await relaunchApp(); - }, 250); - } + let timer: ReturnType; + if (isRestarting) { + timer = setTimeout(async () => { + await relaunchApp(); + }, 250); + } + return () => { + if (timer) clearTimeout(timer); }; - // eslint-disable-next-line @typescript-eslint/no-floating-promises - doRelaunch(); }, [isRestarting]); + const handleExit = useCallback(() => { + setExiting(true); + // Give time for the UI to render the exiting message + setTimeout(async () => { + await runExitCleanup(); + process.exit(ExitCodes.FATAL_CANCELLATION_ERROR); + }, 100); + }, []); + useKeypress( (key) => { if (key.name === 'escape') { - setExiting(true); - setTimeout(async () => { - await runExitCleanup(); - process.exit(ExitCodes.FATAL_CANCELLATION_ERROR); - }, 100); + handleExit(); } }, { isActive: !isRestarting }, diff --git a/packages/cli/src/ui/hooks/useFolderTrust.test.ts b/packages/cli/src/ui/hooks/useFolderTrust.test.ts index c24133aba7..4c8549ab2c 100644 --- a/packages/cli/src/ui/hooks/useFolderTrust.test.ts +++ b/packages/cli/src/ui/hooks/useFolderTrust.test.ts @@ -4,7 +4,16 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { vi, type Mock, type MockInstance } from 'vitest'; +import { + vi, + describe, + it, + expect, + beforeEach, + afterEach, + type Mock, + type MockInstance, +} from 'vitest'; import { act } from 'react'; import { renderHook } from '../../test-utils/render.js'; import { waitFor } from '../../test-utils/async.js'; @@ -118,7 +127,7 @@ describe('useFolderTrust', () => { expect(addItem).not.toHaveBeenCalled(); }); - it('should handle TRUST_FOLDER choice', async () => { + it('should handle TRUST_FOLDER choice and trigger restart', async () => { isWorkspaceTrustedSpy.mockReturnValue({ isTrusted: undefined, source: undefined, @@ -148,12 +157,13 @@ describe('useFolderTrust', () => { '/test/path', TrustLevel.TRUST_FOLDER, ); - expect(result.current.isFolderTrustDialogOpen).toBe(false); + expect(result.current.isRestarting).toBe(true); + expect(result.current.isFolderTrustDialogOpen).toBe(true); expect(onTrustChange).toHaveBeenLastCalledWith(true); }); }); - it('should handle TRUST_PARENT choice', () => { + it('should handle TRUST_PARENT choice and trigger restart', async () => { isWorkspaceTrustedSpy.mockReturnValue({ isTrusted: undefined, source: undefined, @@ -162,19 +172,22 @@ describe('useFolderTrust', () => { useFolderTrust(mockSettings, onTrustChange, addItem), ); - act(() => { + await act(async () => { result.current.handleFolderTrustSelect(FolderTrustChoice.TRUST_PARENT); }); - expect(mockTrustedFolders.setValue).toHaveBeenCalledWith( - '/test/path', - TrustLevel.TRUST_PARENT, - ); - expect(result.current.isFolderTrustDialogOpen).toBe(false); - expect(onTrustChange).toHaveBeenLastCalledWith(true); + await waitFor(() => { + expect(mockTrustedFolders.setValue).toHaveBeenCalledWith( + '/test/path', + TrustLevel.TRUST_PARENT, + ); + expect(result.current.isRestarting).toBe(true); + expect(result.current.isFolderTrustDialogOpen).toBe(true); + expect(onTrustChange).toHaveBeenLastCalledWith(true); + }); }); - it('should handle DO_NOT_TRUST choice and trigger restart', () => { + it('should handle DO_NOT_TRUST choice and NOT trigger restart (implicit -> explicit)', async () => { isWorkspaceTrustedSpy.mockReturnValue({ isTrusted: undefined, source: undefined, @@ -183,17 +196,19 @@ describe('useFolderTrust', () => { useFolderTrust(mockSettings, onTrustChange, addItem), ); - act(() => { + await act(async () => { result.current.handleFolderTrustSelect(FolderTrustChoice.DO_NOT_TRUST); }); - expect(mockTrustedFolders.setValue).toHaveBeenCalledWith( - '/test/path', - TrustLevel.DO_NOT_TRUST, - ); - expect(onTrustChange).toHaveBeenLastCalledWith(false); - expect(result.current.isRestarting).toBe(true); - expect(result.current.isFolderTrustDialogOpen).toBe(true); + await waitFor(() => { + expect(mockTrustedFolders.setValue).toHaveBeenCalledWith( + '/test/path', + TrustLevel.DO_NOT_TRUST, + ); + expect(onTrustChange).toHaveBeenLastCalledWith(false); + expect(result.current.isRestarting).toBe(false); + expect(result.current.isFolderTrustDialogOpen).toBe(false); + }); }); it('should do nothing for default choice', async () => { @@ -205,7 +220,7 @@ describe('useFolderTrust', () => { useFolderTrust(mockSettings, onTrustChange, addItem), ); - act(() => { + await act(async () => { result.current.handleFolderTrustSelect( 'invalid_choice' as FolderTrustChoice, ); @@ -237,7 +252,7 @@ describe('useFolderTrust', () => { expect(result.current.isTrusted).toBe(false); }); - act(() => { + await act(async () => { result.current.handleFolderTrustSelect(FolderTrustChoice.TRUST_FOLDER); }); @@ -247,21 +262,23 @@ describe('useFolderTrust', () => { }); }); - it('should not set isRestarting to true when trust status does not change', () => { + it('should not set isRestarting to true when trust status does not change (true -> true)', async () => { isWorkspaceTrustedSpy.mockReturnValue({ - isTrusted: undefined, - source: undefined, + isTrusted: true, + source: 'file', }); const { result } = renderHook(() => useFolderTrust(mockSettings, onTrustChange, addItem), ); - act(() => { + await act(async () => { result.current.handleFolderTrustSelect(FolderTrustChoice.TRUST_FOLDER); }); - expect(result.current.isRestarting).toBe(false); - expect(result.current.isFolderTrustDialogOpen).toBe(false); // Dialog should close + await waitFor(() => { + expect(result.current.isRestarting).toBe(false); + expect(result.current.isFolderTrustDialogOpen).toBe(false); // Dialog should close + }); }); it('should emit feedback on failure to set value', async () => { diff --git a/packages/cli/src/ui/hooks/useFolderTrust.ts b/packages/cli/src/ui/hooks/useFolderTrust.ts index aae18e4452..b975319c83 100644 --- a/packages/cli/src/ui/hooks/useFolderTrust.ts +++ b/packages/cli/src/ui/hooks/useFolderTrust.ts @@ -49,25 +49,17 @@ export const useFolderTrust = ( const handleFolderTrustSelect = useCallback( (choice: FolderTrustChoice) => { - const trustedFolders = loadTrustedFolders(); + const trustLevelMap: Record = { + [FolderTrustChoice.TRUST_FOLDER]: TrustLevel.TRUST_FOLDER, + [FolderTrustChoice.TRUST_PARENT]: TrustLevel.TRUST_PARENT, + [FolderTrustChoice.DO_NOT_TRUST]: TrustLevel.DO_NOT_TRUST, + }; + + const trustLevel = trustLevelMap[choice]; + if (!trustLevel) return; + const cwd = process.cwd(); - let trustLevel: TrustLevel; - - const wasTrusted = isTrusted ?? true; - - switch (choice) { - case FolderTrustChoice.TRUST_FOLDER: - trustLevel = TrustLevel.TRUST_FOLDER; - break; - case FolderTrustChoice.TRUST_PARENT: - trustLevel = TrustLevel.TRUST_PARENT; - break; - case FolderTrustChoice.DO_NOT_TRUST: - trustLevel = TrustLevel.DO_NOT_TRUST; - break; - default: - return; - } + const trustedFolders = loadTrustedFolders(); try { trustedFolders.setValue(cwd, trustLevel); @@ -86,11 +78,15 @@ export const useFolderTrust = ( const currentIsTrusted = trustLevel === TrustLevel.TRUST_FOLDER || trustLevel === TrustLevel.TRUST_PARENT; - setIsTrusted(currentIsTrusted); - onTrustChange(currentIsTrusted); - const needsRestart = wasTrusted !== currentIsTrusted; - if (needsRestart) { + onTrustChange(currentIsTrusted); + setIsTrusted(currentIsTrusted); + + // logic: we restart if the trust state *effectively* changes from the previous state. + // previous state was `isTrusted`. If undefined, we assume false (untrusted). + const wasTrusted = isTrusted ?? false; + + if (wasTrusted !== currentIsTrusted) { setIsRestarting(true); setIsFolderTrustDialogOpen(true); } else { diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index aceea0efef..bf16680f44 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -1428,22 +1428,13 @@ export class Config { * 'false' for untrusted. */ isTrustedFolder(): boolean { - // isWorkspaceTrusted in cli/src/config/trustedFolder.js returns undefined - // when the file based trust value is unavailable, since it is mainly used - // in the initialization for trust dialogs, etc. Here we return true since - // config.isTrustedFolder() is used for the main business logic of blocking - // tool calls etc in the rest of the application. - // - // Default value is true since we load with trusted settings to avoid - // restarts in the more common path. If the user chooses to mark the folder - // as untrusted, the CLI will restart and we will have the trust value - // reloaded. const context = ideContextStore.get(); if (context?.workspaceState?.isTrusted !== undefined) { return context.workspaceState.isTrusted; } - return this.trustedFolder ?? true; + // Default to untrusted if folder trust is enabled and no explicit value is set. + return this.folderTrust ? (this.trustedFolder ?? false) : true; } setIdeMode(value: boolean): void {