diff --git a/packages/cli/src/ui/hooks/useGitBranchName.test.ts b/packages/cli/src/ui/hooks/useGitBranchName.test.ts index fd43ac76581..54bb26f51ae 100644 --- a/packages/cli/src/ui/hooks/useGitBranchName.test.ts +++ b/packages/cli/src/ui/hooks/useGitBranchName.test.ts @@ -26,32 +26,27 @@ vi.mock('@google/gemini-cli-core', async () => { // Mock fs and fs/promises vi.mock('node:fs', async () => { const memfs = await vi.importActual('memfs'); - return { - ...memfs.fs, - default: memfs.fs, - }; + return { ...memfs.fs, default: memfs.fs }; }); vi.mock('node:fs/promises', async () => { const memfs = await vi.importActual('memfs'); - return memfs.fs.promises; + return { ...memfs.fs.promises, default: memfs.fs.promises }; }); const CWD = '/test/project'; -const GIT_LOGS_HEAD_PATH = `${CWD}/.git/logs/HEAD`; +const GIT_HEAD_PATH = `${CWD}/.git/HEAD`; describe('useGitBranchName', () => { beforeEach(() => { vol.reset(); // Reset in-memory filesystem vol.fromJSON({ - [GIT_LOGS_HEAD_PATH]: 'ref: refs/heads/main', + [GIT_HEAD_PATH]: 'ref: refs/heads/main', }); - vi.useFakeTimers(); // Use fake timers for async operations }); afterEach(() => { vi.restoreAllMocks(); - vi.clearAllTimers(); }); it('should return branch name', async () => { @@ -60,14 +55,11 @@ describe('useGitBranchName', () => { stdout: 'main\n', } as { stdout: string; stderr: string }, ); - const { result, rerender } = renderHook(() => useGitBranchName(CWD)); + const { result } = renderHook(() => useGitBranchName(CWD)); - await act(async () => { - vi.runAllTimers(); // Advance timers to trigger useEffect and exec callback - rerender(); // Rerender to get the updated state + await waitFor(() => { + expect(result.current).toBe('main'); }); - - expect(result.current).toBe('main'); }); it('should return undefined if git command fails', async () => { @@ -75,14 +67,11 @@ describe('useGitBranchName', () => { new Error('Git error'), ); - const { result, rerender } = renderHook(() => useGitBranchName(CWD)); - expect(result.current).toBeUndefined(); + const { result } = renderHook(() => useGitBranchName(CWD)); - await act(async () => { - vi.runAllTimers(); - rerender(); + await waitFor(() => { + expect(result.current).toBeUndefined(); }); - expect(result.current).toBeUndefined(); }); it('should return short commit hash if branch is HEAD (detached state)', async () => { @@ -97,12 +86,10 @@ describe('useGitBranchName', () => { return { stdout: '' } as { stdout: string; stderr: string }; }); - const { result, rerender } = renderHook(() => useGitBranchName(CWD)); - await act(async () => { - vi.runAllTimers(); - rerender(); + const { result } = renderHook(() => useGitBranchName(CWD)); + await waitFor(() => { + expect(result.current).toBe('a1b2c3d'); }); - expect(result.current).toBe('a1b2c3d'); }); it('should return undefined if branch is HEAD and getting commit hash fails', async () => { @@ -117,16 +104,24 @@ describe('useGitBranchName', () => { return { stdout: '' } as { stdout: string; stderr: string }; }); - const { result, rerender } = renderHook(() => useGitBranchName(CWD)); - await act(async () => { - vi.runAllTimers(); - rerender(); + const { result } = renderHook(() => useGitBranchName(CWD)); + await waitFor(() => { + expect(result.current).toBeUndefined(); }); - expect(result.current).toBeUndefined(); }); - it('should update branch name when .git/HEAD changes', async ({ skip }) => { - skip(); // TODO: fix + it('should update branch name when .git/HEAD changes', async () => { + let watcherCallback: + | ((event: string, filename: string | null) => void) + | undefined; + const closeMock = vi.fn(); + const watchMock = vi.spyOn(fs, 'watch').mockImplementation((_path, cb) => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + watcherCallback = cb as any; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return { close: closeMock } as any; + }); + (mockSpawnAsync as MockedFunction) .mockResolvedValueOnce({ stdout: 'main\n' } as { stdout: string; @@ -137,30 +132,36 @@ describe('useGitBranchName', () => { stderr: string; }); - const { result, rerender } = renderHook(() => useGitBranchName(CWD)); + const { result } = renderHook(() => useGitBranchName(CWD)); - await act(async () => { - vi.runAllTimers(); - rerender(); + // Wait for initial fetch and watcher setup + await waitFor(() => { + expect(result.current).toBe('main'); + expect(watchMock).toHaveBeenCalledWith( + GIT_HEAD_PATH, + expect.any(Function), + ); }); - expect(result.current).toBe('main'); + expect(mockSpawnAsync).toHaveBeenCalledTimes(1); - // Simulate file change event - // Ensure the watcher is set up before triggering the change - await act(async () => { - fs.writeFileSync(GIT_LOGS_HEAD_PATH, 'ref: refs/heads/develop'); // Trigger watcher - vi.runAllTimers(); // Process timers for watcher and exec - rerender(); + // Simulate file change event by calling the captured callback + act(() => { + expect(watcherCallback).toBeDefined(); + if (watcherCallback) { + watcherCallback('change', null); + } }); + // Wait for the branch name to update await waitFor(() => { expect(result.current).toBe('develop'); }); + expect(mockSpawnAsync).toHaveBeenCalledTimes(2); }); it('should handle watcher setup error silently', async () => { - // Remove .git/logs/HEAD to cause an error in fs.watch setup - vol.unlinkSync(GIT_LOGS_HEAD_PATH); + // Remove .git/HEAD to cause an error in fs.watch setup + vol.unlinkSync(GIT_HEAD_PATH); (mockSpawnAsync as MockedFunction).mockResolvedValue( { @@ -168,40 +169,31 @@ describe('useGitBranchName', () => { } as { stdout: string; stderr: string }, ); - const { result, rerender } = renderHook(() => useGitBranchName(CWD)); + const { result } = renderHook(() => useGitBranchName(CWD)); - await act(async () => { - vi.runAllTimers(); - rerender(); + await waitFor(() => { + expect(result.current).toBe('main'); // Branch name should still be fetched initially }); - - expect(result.current).toBe('main'); // Branch name should still be fetched initially - - ( - mockSpawnAsync as MockedFunction - ).mockResolvedValueOnce({ - stdout: 'develop\n', - } as { stdout: string; stderr: string }); + expect(mockSpawnAsync).toHaveBeenCalledTimes(1); // This write would trigger the watcher if it was set up // but since it failed, the branch name should not update // We need to create the file again for writeFileSync to not throw vol.fromJSON({ - [GIT_LOGS_HEAD_PATH]: 'ref: refs/heads/develop', + [GIT_HEAD_PATH]: 'ref: refs/heads/develop', }); - await act(async () => { - fs.writeFileSync(GIT_LOGS_HEAD_PATH, 'ref: refs/heads/develop'); - vi.runAllTimers(); - rerender(); + act(() => { + fs.writeFileSync(GIT_HEAD_PATH, 'ref: refs/heads/develop'); }); - // Branch name should not change because watcher setup failed + // Branch name should not change because watcher setup failed, + // and spawnAsync should not have been called again. + expect(mockSpawnAsync).toHaveBeenCalledTimes(1); expect(result.current).toBe('main'); }); - it('should cleanup watcher on unmount', async ({ skip }) => { - skip(); // TODO: fix + it('should cleanup watcher on unmount', async () => { const closeMock = vi.fn(); const watchMock = vi.spyOn(fs, 'watch').mockReturnValue({ close: closeMock, @@ -213,18 +205,18 @@ describe('useGitBranchName', () => { } as { stdout: string; stderr: string }, ); - const { unmount, rerender } = renderHook(() => useGitBranchName(CWD)); + const { unmount } = renderHook(() => useGitBranchName(CWD)); - await act(async () => { - vi.runAllTimers(); - rerender(); + // Wait for the watcher to be set up + await waitFor(() => { + expect(watchMock).toHaveBeenCalledWith( + GIT_HEAD_PATH, + expect.any(Function), + ); }); unmount(); - expect(watchMock).toHaveBeenCalledWith( - GIT_LOGS_HEAD_PATH, - expect.any(Function), - ); + expect(closeMock).toHaveBeenCalled(); }); }); diff --git a/packages/cli/src/ui/hooks/useGitBranchName.ts b/packages/cli/src/ui/hooks/useGitBranchName.ts index d56af9a32f0..2fe4f2f3617 100644 --- a/packages/cli/src/ui/hooks/useGitBranchName.ts +++ b/packages/cli/src/ui/hooks/useGitBranchName.ts @@ -4,14 +4,158 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { useState, useEffect, useCallback } from 'react'; +import { useState, useEffect, useCallback, useRef } from 'react'; import { spawnAsync } from '@google/gemini-cli-core'; import fs from 'node:fs'; import fsPromises from 'node:fs/promises'; import path from 'node:path'; +// A hook that watches for changes in the Git repository's HEAD, handling +// repository initialization after the hook is active. +// @param cwd The current working directory. +// @param onBranchChange A callback to be invoked when a branch change is detected. +function useGitWatcher(cwd: string, onBranchChange: () => void) { + // This effect implements a two-step strategy to watch for Git branch changes + // in a way that is both efficient and handles the edge case of a Git + // repository being initialized after the CLI has started. + // + // 1. Watch .git/HEAD directly: If the repository exists on startup, + // we watch the .git/HEAD file for changes. This is the most direct and + // efficient method, as this file is updated whenever the branch changes. + // + // 2. Watch for .git directory creation: If .git/HEAD doesn't exist at + // startup, we fall back to watching the current working directory (cwd) + // for the creation of the .git directory. This handles the case where the + // user runs `git init` after starting the CLI. Once the .git directory + // is detected, we switch to watching .git/HEAD as in step 1. + useEffect(() => { + onBranchChange(); // Initial fetch + + const gitHeadPath = path.join(cwd, '.git', 'HEAD'); + let headWatcher: fs.FSWatcher | undefined; + let cwdWatcher: fs.FSWatcher | undefined; + + const setupHeadWatcher = () => { + // Clean up existing watchers to prevent resource leaks. + headWatcher?.close(); + // Stop watching the cwd for .git creation if we are now watching HEAD. + cwdWatcher?.close(); + cwdWatcher = undefined; + + try { + console.debug('[GitBranchName] Setting up HEAD watcher.'); + headWatcher = fs.watch(gitHeadPath, (eventType: string) => { + if (eventType === 'rename') { + // The file might have been deleted. + void (async () => { + try { + await fsPromises.access(gitHeadPath, fs.constants.F_OK); + // File still exists, so it was likely an atomic write. + // Trigger a branch change check. + onBranchChange(); + } catch { + // File is gone. Fall back to watching the CWD. + console.debug( + '[GitBranchName] .git/HEAD deleted. Falling back to CWD watcher.', + ); + // The headWatcher is now invalid, so close it before setting up the new one. + headWatcher?.close(); + setupCwdWatcher(); + } + })(); + } else if (eventType === 'change') { + // The file content changed. + onBranchChange(); + } + }); + } catch (e) { + // Ignore errors, which can happen if the file is deleted. + console.debug( + '[GitBranchName] Error setting up HEAD watcher. Ignoring.', + e, + ); + } + }; + + const setupCwdWatcher = () => { + // Clean up existing watcher to prevent resource leaks. + cwdWatcher?.close(); + try { + console.debug( + '[GitBranchName] .git/HEAD not found. Setting up CWD watcher.', + ); + cwdWatcher = fs.watch(cwd, (eventType, filename) => { + // On macOS, filename can be null for rename events. + // To be robust, we check for the existence of the .git directory + // if the filename is '.git' or if it's null. + if ( + (eventType === 'rename' || eventType === 'change') && + (filename === '.git' || filename === null) + ) { + // Check if the created .git is actually a directory. + fs.promises + .stat(path.join(cwd, '.git')) + .then((stats) => { + if (stats.isDirectory()) { + console.debug( + '[GitBranchName] .git directory detected. Switching to HEAD watcher.', + ); + // .git directory was created. Try to set up the HEAD watcher. + onBranchChange(); + setupHeadWatcher(); + } else { + console.debug( + '[GitBranchName] .git was found, but it is not a directory. Ignoring.', + ); + } + }) + .catch((err) => { + // Ignore stat errors (e.g. file not found if the change was not .git creation) + console.debug( + '[GitBranchName] Error checking .git status. Ignoring.', + err, + ); + }); + } + }); + } catch (e) { + // Ignore errors. + console.debug( + '[GitBranchName] Error setting up CWD watcher. Ignoring.', + e, + ); + } + }; + + void (async () => { + try { + await fsPromises.access(gitHeadPath, fs.constants.F_OK); + // .git/HEAD exists, watch it directly. + setupHeadWatcher(); + } catch { + // .git/HEAD does not exist, watch the cwd for .git creation. + setupCwdWatcher(); + } + })(); + + return () => { + console.debug('[GitBranchName] Closing watchers.'); + headWatcher?.close(); + cwdWatcher?.close(); + }; + }, [cwd, onBranchChange]); +} + export function useGitBranchName(cwd: string): string | undefined { const [branchName, setBranchName] = useState(undefined); + const isMounted = useRef(true); + + useEffect(() => { + isMounted.current = true; + return () => { + isMounted.current = false; + }; + }, []); const fetchBranchName = useCallback(async () => { try { @@ -20,6 +164,8 @@ export function useGitBranchName(cwd: string): string | undefined { ['rev-parse', '--abbrev-ref', 'HEAD'], { cwd }, ); + if (!isMounted.current) return; + const branch = stdout.toString().trim(); if (branch && branch !== 'HEAD') { setBranchName(branch); @@ -29,43 +175,16 @@ export function useGitBranchName(cwd: string): string | undefined { ['rev-parse', '--short', 'HEAD'], { cwd }, ); + if (!isMounted.current) return; setBranchName(hashStdout.toString().trim()); } } catch (_error) { + if (!isMounted.current) return; setBranchName(undefined); } }, [cwd, setBranchName]); - useEffect(() => { - fetchBranchName(); // Initial fetch - - const gitLogsHeadPath = path.join(cwd, '.git', 'logs', 'HEAD'); - let watcher: fs.FSWatcher | undefined; - - const setupWatcher = async () => { - try { - // Check if .git/logs/HEAD exists, as it might not in a new repo or orphaned head - await fsPromises.access(gitLogsHeadPath, fs.constants.F_OK); - watcher = fs.watch(gitLogsHeadPath, (eventType: string) => { - // Changes to .git/logs/HEAD (appends) indicate HEAD has likely changed - if (eventType === 'change' || eventType === 'rename') { - // Handle rename just in case - fetchBranchName(); - } - }); - } catch (_watchError) { - // Silently ignore watcher errors (e.g. permissions or file not existing), - // similar to how exec errors are handled. - // The branch name will simply not update automatically. - } - }; - - setupWatcher(); - - return () => { - watcher?.close(); - }; - }, [cwd, fetchBranchName]); + useGitWatcher(cwd, fetchBranchName); return branchName; }