fix(cli): ensure branch indicator updates in sub-directories and worktrees (#26330)

This commit is contained in:
Adib234
2026-05-04 13:35:13 -04:00
committed by GitHub
parent 9de8c8aadb
commit 704be5a418
3 changed files with 169 additions and 74 deletions
@@ -12,7 +12,10 @@ import { useGitBranchName } from './useGitBranchName.js';
import { fs, vol } from 'memfs'; import { fs, vol } from 'memfs';
import * as fsPromises from 'node:fs/promises'; import * as fsPromises from 'node:fs/promises';
import path from 'node:path'; // For mocking fs import path from 'node:path'; // For mocking fs
import { spawnAsync as mockSpawnAsync } from '@google/gemini-cli-core'; import {
spawnAsync as mockSpawnAsync,
getAbsoluteGitDir as mockGetAbsoluteGitDir,
} from '@google/gemini-cli-core';
// Mock @google/gemini-cli-core // Mock @google/gemini-cli-core
vi.mock('@google/gemini-cli-core', async () => { vi.mock('@google/gemini-cli-core', async () => {
@@ -22,6 +25,7 @@ vi.mock('@google/gemini-cli-core', async () => {
return { return {
...original, ...original,
spawnAsync: vi.fn(), spawnAsync: vi.fn(),
getAbsoluteGitDir: vi.fn(),
}; };
}); });
@@ -40,19 +44,21 @@ vi.mock('node:fs/promises', async () => {
}); });
const CWD = '/test/project'; const CWD = '/test/project';
const GIT_LOGS_HEAD_PATH = path.join(CWD, '.git', 'logs', 'HEAD'); const GIT_DIR = path.join(CWD, '.git');
const GIT_HEAD_PATH = path.join(GIT_DIR, 'HEAD');
describe('useGitBranchName', () => { describe('useGitBranchName', () => {
let deferredSpawn: Array<{ let deferredSpawn: Array<{
resolve: (val: { stdout: string; stderr: string }) => void; resolve: (val: { stdout: string; stderr: string; code: number }) => void;
reject: (err: Error) => void; reject: (err: Error) => void;
args: string[]; args: string[];
}> = []; }> = [];
beforeEach(() => { beforeEach(() => {
vi.useFakeTimers();
vol.reset(); // Reset in-memory filesystem vol.reset(); // Reset in-memory filesystem
vol.fromJSON({ vol.fromJSON({
[GIT_LOGS_HEAD_PATH]: 'ref: refs/heads/main', [GIT_HEAD_PATH]: 'ref: refs/heads/main',
}); });
deferredSpawn = []; deferredSpawn = [];
@@ -62,9 +68,11 @@ describe('useGitBranchName', () => {
deferredSpawn.push({ resolve, reject, args }); deferredSpawn.push({ resolve, reject, args });
}), }),
); );
vi.mocked(mockGetAbsoluteGitDir).mockResolvedValue(GIT_DIR);
}); });
afterEach(() => { afterEach(() => {
vi.useRealTimers();
vi.restoreAllMocks(); vi.restoreAllMocks();
}); });
@@ -86,16 +94,35 @@ describe('useGitBranchName', () => {
}; };
}; };
/**
* Helper to resolve pending spawns for a hook render.
*/
const resolveInitialSpawns = async (branch: string = 'main') => {
await act(async () => {
let resolvedAny = true;
while (resolvedAny || deferredSpawn.length > 0) {
resolvedAny = false;
while (deferredSpawn.length > 0) {
const spawn = deferredSpawn.shift()!;
if (spawn.args.includes('--abbrev-ref')) {
spawn.resolve({ stdout: `${branch}\n`, stderr: '', code: 0 });
resolvedAny = true;
} else if (spawn.args.includes('--short')) {
spawn.resolve({ stdout: `${branch}\n`, stderr: '', code: 0 });
resolvedAny = true;
}
}
await vi.advanceTimersByTimeAsync(1);
}
});
};
it('should return branch name', async () => { it('should return branch name', async () => {
const { result } = await renderGitBranchNameHook(CWD); const { result } = await renderGitBranchNameHook(CWD);
expect(result.current).toBeUndefined(); expect(result.current).toBeUndefined();
await act(async () => { await resolveInitialSpawns('main');
const spawn = deferredSpawn.shift()!;
expect(spawn.args).toContain('--abbrev-ref');
spawn.resolve({ stdout: 'main\n', stderr: '' });
});
expect(result.current).toBe('main'); expect(result.current).toBe('main');
}); });
@@ -104,9 +131,13 @@ describe('useGitBranchName', () => {
const { result } = await renderGitBranchNameHook(CWD); const { result } = await renderGitBranchNameHook(CWD);
await act(async () => { await act(async () => {
const spawn = deferredSpawn.shift()!; const abbrevSpawn = deferredSpawn.find((s) =>
expect(spawn.args).toContain('--abbrev-ref'); s.args.includes('--abbrev-ref'),
spawn.reject(new Error('Git error')); );
if (abbrevSpawn) {
abbrevSpawn.reject(new Error('Git error'));
}
await vi.advanceTimersByTimeAsync(1);
}); });
expect(result.current).toBeUndefined(); expect(result.current).toBeUndefined();
@@ -116,16 +147,22 @@ describe('useGitBranchName', () => {
const { result } = await renderGitBranchNameHook(CWD); const { result } = await renderGitBranchNameHook(CWD);
await act(async () => { await act(async () => {
const spawn = deferredSpawn.shift()!; const abbrevSpawn = deferredSpawn.find((s) =>
expect(spawn.args).toContain('--abbrev-ref'); s.args.includes('--abbrev-ref'),
spawn.resolve({ stdout: 'HEAD\n', stderr: '' }); )!;
abbrevSpawn.resolve({ stdout: 'HEAD\n', stderr: '', code: 0 });
await vi.advanceTimersByTimeAsync(1);
}); });
// It should now call spawnAsync again for the short hash // It should now call spawnAsync again for the short hash
await act(async () => { await act(async () => {
const spawn = deferredSpawn.shift()!; const shortSpawn = deferredSpawn.find((s) => s.args.includes('--short'));
expect(spawn.args).toContain('--short'); if (shortSpawn) {
spawn.resolve({ stdout: 'a1b2c3d\n', stderr: '' }); shortSpawn.resolve({ stdout: 'a1b2c3d\n', stderr: '', code: 0 });
} else {
throw new Error('Short spawn not found');
}
await vi.advanceTimersByTimeAsync(1);
}); });
expect(result.current).toBe('a1b2c3d'); expect(result.current).toBe('a1b2c3d');
@@ -135,15 +172,21 @@ describe('useGitBranchName', () => {
const { result } = await renderGitBranchNameHook(CWD); const { result } = await renderGitBranchNameHook(CWD);
await act(async () => { await act(async () => {
const spawn = deferredSpawn.shift()!; const abbrevSpawn = deferredSpawn.find((s) =>
expect(spawn.args).toContain('--abbrev-ref'); s.args.includes('--abbrev-ref'),
spawn.resolve({ stdout: 'HEAD\n', stderr: '' }); )!;
abbrevSpawn.resolve({ stdout: 'HEAD\n', stderr: '', code: 0 });
await vi.advanceTimersByTimeAsync(1);
}); });
await act(async () => { await act(async () => {
const spawn = deferredSpawn.shift()!; const shortSpawn = deferredSpawn.find((s) => s.args.includes('--short'));
expect(spawn.args).toContain('--short'); if (shortSpawn) {
spawn.reject(new Error('Git error')); shortSpawn.reject(new Error('Git error'));
} else {
throw new Error('Short spawn not found');
}
await vi.advanceTimersByTimeAsync(1);
}); });
expect(result.current).toBeUndefined(); expect(result.current).toBeUndefined();
@@ -151,64 +194,94 @@ describe('useGitBranchName', () => {
it('should update branch name when .git/HEAD changes', async () => { it('should update branch name when .git/HEAD changes', async () => {
vi.spyOn(fsPromises, 'access').mockResolvedValue(undefined); vi.spyOn(fsPromises, 'access').mockResolvedValue(undefined);
const watchSpy = vi.spyOn(fs, 'watch'); let watchCallback:
| ((eventType: string, filename: string | null) => void)
| undefined;
const watchSpy = vi.spyOn(fs, 'watch').mockImplementation(((
_path: string,
callback: (eventType: string, filename: string | null) => void,
) => {
watchCallback = callback;
return { close: vi.fn() };
}) as unknown as typeof fs.watch);
const { result } = await renderGitBranchNameHook(CWD); const { result } = await renderGitBranchNameHook(CWD);
await act(async () => { await resolveInitialSpawns('main');
const spawn = deferredSpawn.shift()!;
expect(spawn.args).toContain('--abbrev-ref');
spawn.resolve({ stdout: 'main\n', stderr: '' });
});
expect(result.current).toBe('main'); expect(result.current).toBe('main');
// Wait for watcher to be set up // Wait for watcher to be set up
await waitFor(() => { await waitFor(() => {
expect(watchSpy).toHaveBeenCalled(); expect(watchSpy).toHaveBeenCalledWith(GIT_DIR, expect.any(Function));
}); });
// Simulate file change event // Simulate file change event for HEAD
await act(async () => { await act(async () => {
fs.writeFileSync(GIT_LOGS_HEAD_PATH, 'ref: refs/heads/develop'); // Trigger watcher if (watchCallback) {
watchCallback('change', 'HEAD');
}
await vi.advanceTimersByTimeAsync(150); // triggers debounce
}); });
// Resolving the new branch name fetch // Resolving the new branch name fetch
await act(async () => { await act(async () => {
const spawn = deferredSpawn.shift()!; // Find the specific abbrev-ref spawn for this update
expect(spawn.args).toContain('--abbrev-ref'); const spawn = deferredSpawn.find((s) => s.args.includes('--abbrev-ref'))!;
spawn.resolve({ stdout: 'develop\n', stderr: '' }); // Remove it from the array so subsequent lookups don't find the same one
deferredSpawn.splice(deferredSpawn.indexOf(spawn), 1);
spawn.resolve({ stdout: 'develop\n', stderr: '', code: 0 });
await vi.advanceTimersByTimeAsync(1);
}); });
expect(result.current).toBe('develop'); expect(result.current).toBe('develop');
// Simulate file change event with null filename (platform compatibility)
await act(async () => {
if (watchCallback) {
watchCallback('change', null);
}
await vi.advanceTimersByTimeAsync(150);
});
// Resolving the new branch name fetch
await act(async () => {
const spawn = deferredSpawn.find((s) => s.args.includes('--abbrev-ref'))!;
deferredSpawn.splice(deferredSpawn.indexOf(spawn), 1);
spawn.resolve({ stdout: 'feature-x\n', stderr: '', code: 0 });
await vi.advanceTimersByTimeAsync(1);
});
expect(result.current).toBe('feature-x');
}); });
it('should handle watcher setup error silently', async () => { it('should handle watcher setup error silently', async () => {
// Remove .git/logs/HEAD to cause an error in fs.watch setup // Cause an error in absolute git dir setup
vol.unlinkSync(GIT_LOGS_HEAD_PATH); vi.mocked(mockGetAbsoluteGitDir).mockRejectedValueOnce(
new Error('Git error'),
);
const { result } = await renderGitBranchNameHook(CWD); const { result } = await renderGitBranchNameHook(CWD);
await act(async () => { await act(async () => {
const spawn = deferredSpawn.shift()!; const spawn = deferredSpawn.shift()!;
expect(spawn.args).toContain('--abbrev-ref'); expect(spawn.args).toContain('--abbrev-ref');
spawn.resolve({ stdout: 'main\n', stderr: '' }); spawn.resolve({ stdout: 'main\n', stderr: '', code: 0 });
await vi.advanceTimersByTimeAsync(1);
}); });
expect(result.current).toBe('main'); expect(result.current).toBe('main');
// This write would trigger the watcher if it was set up // Trigger a mock write that would normally be watched
// We need to create the file again for writeFileSync to not throw
vol.fromJSON({
[GIT_LOGS_HEAD_PATH]: 'ref: refs/heads/develop',
});
await act(async () => { await act(async () => {
fs.writeFileSync(GIT_LOGS_HEAD_PATH, 'ref: refs/heads/develop'); fs.writeFileSync(GIT_HEAD_PATH, 'ref: refs/heads/develop');
await vi.advanceTimersByTimeAsync(1);
}); });
// spawnAsync should NOT have been called again for updating // spawnAsync should NOT have been called again for updating
expect(deferredSpawn.length).toBe(0); expect(
deferredSpawn.filter((s) => s.args.includes('--abbrev-ref')).length,
).toBe(0);
expect(result.current).toBe('main'); expect(result.current).toBe('main');
}); });
@@ -221,18 +294,11 @@ describe('useGitBranchName', () => {
const { unmount } = await renderGitBranchNameHook(CWD); const { unmount } = await renderGitBranchNameHook(CWD);
await act(async () => { await resolveInitialSpawns('main');
const spawn = deferredSpawn.shift()!;
expect(spawn.args).toContain('--abbrev-ref');
spawn.resolve({ stdout: 'main\n', stderr: '' });
});
// Wait for watcher to be set up BEFORE unmounting // Wait for watcher to be set up BEFORE unmounting
await waitFor(() => { await waitFor(() => {
expect(watchMock).toHaveBeenCalledWith( expect(watchMock).toHaveBeenCalledWith(GIT_DIR, expect.any(Function));
GIT_LOGS_HEAD_PATH,
expect.any(Function),
);
}); });
unmount(); unmount();
+35 -18
View File
@@ -4,14 +4,14 @@
* SPDX-License-Identifier: Apache-2.0 * 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 { spawnAsync, getAbsoluteGitDir } from '@google/gemini-cli-core';
import fs from 'node:fs'; import fs from 'node:fs';
import fsPromises from 'node:fs/promises'; import fsPromises from 'node:fs/promises';
import path from 'node:path';
export function useGitBranchName(cwd: string): string | undefined { export function useGitBranchName(cwd: string): string | undefined {
const [branchName, setBranchName] = useState<string | undefined>(undefined); const [branchName, setBranchName] = useState<string | undefined>(undefined);
const timeoutRef = useRef<NodeJS.Timeout | null>(null);
const fetchBranchName = useCallback(async () => { const fetchBranchName = useCallback(async () => {
try { try {
@@ -37,26 +37,41 @@ export function useGitBranchName(cwd: string): string | undefined {
}, [cwd, setBranchName]); }, [cwd, setBranchName]);
useEffect(() => { useEffect(() => {
// eslint-disable-next-line @typescript-eslint/no-floating-promises void fetchBranchName(); // Initial fetch
fetchBranchName(); // Initial fetch
const gitLogsHeadPath = path.join(cwd, '.git', 'logs', 'HEAD');
let watcher: fs.FSWatcher | undefined; let watcher: fs.FSWatcher | undefined;
let cancelled = false; let cancelled = false;
const setupWatcher = async () => { const setupWatcher = async () => {
try { try {
// Check if .git/logs/HEAD exists, as it might not in a new repo or orphaned head const gitDir = await getAbsoluteGitDir(cwd);
await fsPromises.access(gitLogsHeadPath, fs.constants.F_OK); if (!gitDir) return;
// Ensure we can access the git dir
await fsPromises.access(gitDir, fs.constants.F_OK);
if (cancelled) return; if (cancelled) return;
watcher = fs.watch(gitLogsHeadPath, (eventType: string) => {
// Changes to .git/logs/HEAD (appends) indicate HEAD has likely changed const w = fs.watch(
if (eventType === 'change' || eventType === 'rename') { gitDir,
// Handle rename just in case (eventType: string, filename: string | null) => {
// eslint-disable-next-line @typescript-eslint/no-floating-promises // Changes to HEAD indicate branch checkout or detached commit.
fetchBranchName(); // On some platforms filename may be null, so we refresh in that case too.
} if (!filename || filename === 'HEAD') {
}); if (timeoutRef.current) {
clearTimeout(timeoutRef.current);
}
timeoutRef.current = setTimeout(() => {
void fetchBranchName();
}, 100);
}
},
);
if (cancelled) {
w.close();
} else {
watcher = w;
}
} catch { } catch {
// Silently ignore watcher errors (e.g. permissions or file not existing), // Silently ignore watcher errors (e.g. permissions or file not existing),
// similar to how exec errors are handled. // similar to how exec errors are handled.
@@ -64,11 +79,13 @@ export function useGitBranchName(cwd: string): string | undefined {
} }
}; };
// eslint-disable-next-line @typescript-eslint/no-floating-promises void setupWatcher();
setupWatcher();
return () => { return () => {
cancelled = true; cancelled = true;
if (timeoutRef.current) {
clearTimeout(timeoutRef.current);
}
watcher?.close(); watcher?.close();
}; };
}, [cwd, fetchBranchName]); }, [cwd, fetchBranchName]);
+12
View File
@@ -6,6 +6,18 @@
import * as fs from 'node:fs'; import * as fs from 'node:fs';
import * as path from 'node:path'; import * as path from 'node:path';
import { spawnAsync } from './shell-utils.js';
/**
* Gets the absolute path to the git directory (.git) for the given working directory.
* This handles standard git repositories, subdirectories, and worktrees.
*/
export async function getAbsoluteGitDir(cwd: string): Promise<string> {
const result = await spawnAsync('git', ['rev-parse', '--absolute-git-dir'], {
cwd,
});
return result.stdout.trim();
}
/** /**
* Checks if a directory is within a git repository * Checks if a directory is within a git repository