diff --git a/packages/cli/src/ui/hooks/useFolderTrust.test.ts b/packages/cli/src/ui/hooks/useFolderTrust.test.ts index c988fe711a3..010e8281499 100644 --- a/packages/cli/src/ui/hooks/useFolderTrust.test.ts +++ b/packages/cli/src/ui/hooks/useFolderTrust.test.ts @@ -25,7 +25,12 @@ import { type LoadedTrustedFolders, } from '../../config/trustedFolders.js'; import * as trustedFolders from '../../config/trustedFolders.js'; -import { coreEvents, ExitCodes, isHeadlessMode } from '@google/gemini-cli-core'; +import { + coreEvents, + ExitCodes, + isHeadlessMode, + FolderTrustDiscoveryService, +} from '@google/gemini-cli-core'; import { MessageType } from '../types.js'; const mockedCwd = vi.hoisted(() => vi.fn().mockReturnValue('/mock/cwd')); @@ -366,7 +371,7 @@ describe('useFolderTrust', () => { }); describe('headless mode', () => { - it('should force trust and hide dialog in headless mode', async () => { + it('should propagate false to onTrustChange, hide dialog, and show warning when folder is untrusted', async () => { vi.mocked(isHeadlessMode).mockReturnValue(true); isWorkspaceTrustedSpy.mockReturnValue({ isTrusted: false, @@ -378,7 +383,8 @@ describe('useFolderTrust', () => { ); expect(result.current.isFolderTrustDialogOpen).toBe(false); - expect(onTrustChange).toHaveBeenCalledWith(true); + expect(result.current.isTrusted).toBe(false); + expect(onTrustChange).toHaveBeenCalledWith(false); expect(addItem).toHaveBeenCalledWith( expect.objectContaining({ type: MessageType.INFO, @@ -387,5 +393,255 @@ describe('useFolderTrust', () => { expect.any(Number), ); }); + + it('should propagate true to onTrustChange, hide dialog, and not show warning when folder is trusted', async () => { + vi.mocked(isHeadlessMode).mockReturnValue(true); + isWorkspaceTrustedSpy.mockReturnValue({ + isTrusted: true, + source: 'file', + }); + + const { result } = await renderHook(() => + useFolderTrust(mockSettings, onTrustChange, addItem), + ); + + expect(result.current.isFolderTrustDialogOpen).toBe(false); + expect(result.current.isTrusted).toBe(true); + expect(onTrustChange).toHaveBeenCalledWith(true); + expect(addItem).not.toHaveBeenCalled(); + }); + + it('should propagate undefined to onTrustChange and hide dialog when folder trust is undefined', async () => { + vi.mocked(isHeadlessMode).mockReturnValue(true); + isWorkspaceTrustedSpy.mockReturnValue({ + isTrusted: undefined, + source: undefined, + }); + + const { result } = await renderHook(() => + useFolderTrust(mockSettings, onTrustChange, addItem), + ); + + expect(result.current.isFolderTrustDialogOpen).toBe(false); + expect(result.current.isTrusted).toBeUndefined(); + expect(onTrustChange).toHaveBeenCalledWith(undefined); + expect(addItem).not.toHaveBeenCalled(); + }); + }); + + describe('callback stability', () => { + it('should not re-run effect or trigger onTrustChange again when callback references change', async () => { + isWorkspaceTrustedSpy.mockReturnValue({ + isTrusted: true, + source: 'file', + }); + + const initialOnTrustChange = vi.fn(); + const initialAddItem = vi.fn(); + + const { rerender } = await renderHook( + ({ onTrustChangeCb, addItemCb }) => + useFolderTrust(mockSettings, onTrustChangeCb, addItemCb), + { + initialProps: { + onTrustChangeCb: initialOnTrustChange, + addItemCb: initialAddItem, + }, + }, + ); + + expect(initialOnTrustChange).toHaveBeenCalledTimes(1); + expect(initialOnTrustChange).toHaveBeenCalledWith(true); + + const newOnTrustChange = vi.fn(); + const newAddItem = vi.fn(); + + rerender({ + onTrustChangeCb: newOnTrustChange, + addItemCb: newAddItem, + }); + + expect(newOnTrustChange).not.toHaveBeenCalled(); + expect(initialOnTrustChange).toHaveBeenCalledTimes(1); + }); + + it('should not re-trigger FolderTrustDiscoveryService.discover when callback references change', async () => { + isWorkspaceTrustedSpy.mockReturnValue({ + isTrusted: false, + source: 'file', + }); + const discoverSpy = vi.spyOn(FolderTrustDiscoveryService, 'discover'); + discoverSpy.mockClear(); + + const initialOnTrustChange = vi.fn(); + const initialAddItem = vi.fn(); + + const { rerender } = await renderHook( + ({ onTrustChangeCb, addItemCb }) => + useFolderTrust(mockSettings, onTrustChangeCb, addItemCb), + { + initialProps: { + onTrustChangeCb: initialOnTrustChange, + addItemCb: initialAddItem, + }, + }, + ); + + expect(discoverSpy).toHaveBeenCalledTimes(1); + + const newOnTrustChange = vi.fn(); + const newAddItem = vi.fn(); + + rerender({ + onTrustChangeCb: newOnTrustChange, + addItemCb: newAddItem, + }); + + expect(discoverSpy).toHaveBeenCalledTimes(1); + }); + + it('should use updated onTrustChange callback in handleFolderTrustSelect when callback reference changes', async () => { + isWorkspaceTrustedSpy.mockReturnValue({ + isTrusted: undefined, + source: undefined, + }); + + const initialOnTrustChange = vi.fn(); + const initialAddItem = vi.fn(); + + const { result, rerender } = await renderHook( + ({ onTrustChangeCb, addItemCb }) => + useFolderTrust(mockSettings, onTrustChangeCb, addItemCb), + { + initialProps: { + onTrustChangeCb: initialOnTrustChange, + addItemCb: initialAddItem, + }, + }, + ); + + expect(initialOnTrustChange).toHaveBeenCalledWith(undefined); + + const newOnTrustChange = vi.fn(); + const newAddItem = vi.fn(); + + rerender({ + onTrustChangeCb: newOnTrustChange, + addItemCb: newAddItem, + }); + + await act(async () => { + await result.current.handleFolderTrustSelect( + FolderTrustChoice.TRUST_FOLDER, + ); + }); + + expect(newOnTrustChange).toHaveBeenCalledWith(true); + expect(initialOnTrustChange).not.toHaveBeenCalledWith(true); + }); + + it('should not re-run discovery effect when unrelated settings change', async () => { + isWorkspaceTrustedSpy.mockReturnValue({ + isTrusted: true, + source: 'file', + }); + + const onTrustChangeCb = vi.fn(); + const addItemCb = vi.fn(); + + const initialSettings = { + merged: { + security: { + folderTrust: { + enabled: true, + }, + }, + ui: { + theme: 'default', + }, + }, + setValue: vi.fn(), + } as unknown as LoadedSettings; + + const { rerender } = await renderHook( + ({ settings }) => useFolderTrust(settings, onTrustChangeCb, addItemCb), + { + initialProps: { + settings: initialSettings, + }, + }, + ); + + expect(onTrustChangeCb).toHaveBeenCalledTimes(1); + + const updatedSettings = { + merged: { + security: { + folderTrust: { + enabled: true, + }, + }, + ui: { + theme: 'dark', + }, + }, + setValue: vi.fn(), + } as unknown as LoadedSettings; + + rerender({ + settings: updatedSettings, + }); + + expect(onTrustChangeCb).toHaveBeenCalledTimes(1); + }); + + it('should re-run discovery effect when folderTrust setting changes', async () => { + isWorkspaceTrustedSpy.mockReturnValue({ + isTrusted: true, + source: 'file', + }); + + const onTrustChangeCb = vi.fn(); + const addItemCb = vi.fn(); + + const initialSettings = { + merged: { + security: { + folderTrust: { + enabled: true, + }, + }, + }, + setValue: vi.fn(), + } as unknown as LoadedSettings; + + const { rerender } = await renderHook( + ({ settings }) => useFolderTrust(settings, onTrustChangeCb, addItemCb), + { + initialProps: { + settings: initialSettings, + }, + }, + ); + + expect(onTrustChangeCb).toHaveBeenCalledTimes(1); + + const updatedSettings = { + merged: { + security: { + folderTrust: { + enabled: false, + }, + }, + }, + setValue: vi.fn(), + } as unknown as LoadedSettings; + + rerender({ + settings: updatedSettings, + }); + + expect(onTrustChangeCb).toHaveBeenCalledTimes(2); + }); }); }); diff --git a/packages/cli/src/ui/hooks/useFolderTrust.ts b/packages/cli/src/ui/hooks/useFolderTrust.ts index 33c12a110d1..b604ed9b0b3 100644 --- a/packages/cli/src/ui/hooks/useFolderTrust.ts +++ b/packages/cli/src/ui/hooks/useFolderTrust.ts @@ -35,11 +35,23 @@ export const useFolderTrust = ( const [isRestarting, setIsRestarting] = useState(false); const startupMessageSent = useRef(false); + const onTrustChangeRef = useRef(onTrustChange); + const addItemRef = useRef(addItem); + const settingsRef = useRef(settings); + + useEffect(() => { + onTrustChangeRef.current = onTrustChange; + addItemRef.current = addItem; + settingsRef.current = settings; + }, [onTrustChange, addItem, settings]); + const folderTrust = settings.merged.security.folderTrust.enabled ?? true; useEffect(() => { let isMounted = true; - const { isTrusted: trusted } = isWorkspaceTrusted(settings.merged); + const { isTrusted: trusted } = isWorkspaceTrusted( + settingsRef.current.merged, + ); if (trusted === undefined || trusted === false) { void FolderTrustDiscoveryService.discover(process.cwd()) @@ -56,7 +68,7 @@ export const useFolderTrust = ( const showUntrustedMessage = () => { if (trusted === false && !startupMessageSent.current) { - addItem( + addItemRef.current( { type: MessageType.INFO, text: 'This folder is untrusted, project settings, hooks, MCPs, and GEMINI.md files will not be applied for this folder.\nUse the `/permissions` command to change the trust level.', @@ -67,24 +79,17 @@ export const useFolderTrust = ( } }; - if (isHeadlessMode()) { - if (isMounted) { - setIsTrusted(trusted); - setIsFolderTrustDialogOpen(false); - onTrustChange(true); - showUntrustedMessage(); - } - } else if (isMounted) { + if (isMounted) { setIsTrusted(trusted); - setIsFolderTrustDialogOpen(trusted === undefined); - onTrustChange(trusted); + setIsFolderTrustDialogOpen(!isHeadlessMode() && trusted === undefined); + onTrustChangeRef.current(trusted); showUntrustedMessage(); } return () => { isMounted = false; }; - }, [folderTrust, onTrustChange, settings.merged, addItem]); + }, [folderTrust]); const handleFolderTrustSelect = useCallback( async (choice: FolderTrustChoice) => { @@ -118,7 +123,7 @@ export const useFolderTrust = ( trustLevel === TrustLevel.TRUST_FOLDER || trustLevel === TrustLevel.TRUST_PARENT; - onTrustChange(currentIsTrusted); + onTrustChangeRef.current(currentIsTrusted); setIsTrusted(currentIsTrusted); const wasTrusted = isTrusted ?? false; @@ -130,7 +135,7 @@ export const useFolderTrust = ( setIsFolderTrustDialogOpen(false); } }, - [onTrustChange, isTrusted], + [isTrusted], ); return {