From 3e1a377d788ea9ae20cc4cdc7bb6d17a69a32978 Mon Sep 17 00:00:00 2001 From: Jacob Richman Date: Mon, 26 Jan 2026 10:12:40 -0800 Subject: [PATCH] Fix bug in detecting already added paths. (#17430) --- .../src/ui/commands/directoryCommand.test.tsx | 68 ++++++++++++++----- .../cli/src/ui/commands/directoryCommand.tsx | 24 +++++-- 2 files changed, 69 insertions(+), 23 deletions(-) diff --git a/packages/cli/src/ui/commands/directoryCommand.test.tsx b/packages/cli/src/ui/commands/directoryCommand.test.tsx index 673e9805f9..26e9bc727c 100644 --- a/packages/cli/src/ui/commands/directoryCommand.test.tsx +++ b/packages/cli/src/ui/commands/directoryCommand.test.tsx @@ -17,9 +17,18 @@ import type { CommandContext, OpenCustomDialogActionReturn } from './types.js'; import { MessageType } from '../types.js'; import * as os from 'node:os'; import * as path from 'node:path'; +import * as fs from 'node:fs'; import * as trustedFolders from '../../config/trustedFolders.js'; import type { LoadedTrustedFolders } from '../../config/trustedFolders.js'; +vi.mock('node:fs', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + realpathSync: vi.fn((p) => p), + }; +}); + vi.mock('../utils/directoryUtils.js', async (importOriginal) => { const actual = await importOriginal(); @@ -42,13 +51,14 @@ describe('directoryCommand', () => { beforeEach(() => { mockWorkspaceContext = { + targetDir: path.resolve('/test/dir'), addDirectory: vi.fn(), addDirectories: vi.fn().mockReturnValue({ added: [], failed: [] }), getDirectories: vi .fn() .mockReturnValue([ - path.normalize('/home/user/project1'), - path.normalize('/home/user/project2'), + path.resolve('/home/user/project1'), + path.resolve('/home/user/project2'), ]), } as unknown as WorkspaceContext; @@ -58,7 +68,7 @@ describe('directoryCommand', () => { getGeminiClient: vi.fn().mockReturnValue({ addDirectoryContext: vi.fn(), }), - getWorkingDir: () => '/test/dir', + getWorkingDir: () => path.resolve('/test/dir'), shouldLoadMemoryFromIncludeDirectories: () => false, getDebugMode: () => false, getFileService: () => ({}), @@ -91,9 +101,9 @@ describe('directoryCommand', () => { expect(mockContext.ui.addItem).toHaveBeenCalledWith( expect.objectContaining({ type: MessageType.INFO, - text: `Current workspace directories:\n- ${path.normalize( + text: `Current workspace directories:\n- ${path.resolve( '/home/user/project1', - )}\n- ${path.normalize('/home/user/project2')}`, + )}\n- ${path.resolve('/home/user/project2')}`, }), ); }); @@ -125,7 +135,7 @@ describe('directoryCommand', () => { }); it('should call addDirectory and show a success message for a single path', async () => { - const newPath = path.normalize('/home/user/new-project'); + const newPath = path.resolve('/home/user/new-project'); vi.mocked(mockWorkspaceContext.addDirectories).mockReturnValue({ added: [newPath], failed: [], @@ -144,8 +154,8 @@ describe('directoryCommand', () => { }); it('should call addDirectory for each path and show a success message for multiple paths', async () => { - const newPath1 = path.normalize('/home/user/new-project1'); - const newPath2 = path.normalize('/home/user/new-project2'); + const newPath1 = path.resolve('/home/user/new-project1'); + const newPath2 = path.resolve('/home/user/new-project2'); vi.mocked(mockWorkspaceContext.addDirectories).mockReturnValue({ added: [newPath1, newPath2], failed: [], @@ -166,7 +176,7 @@ describe('directoryCommand', () => { it('should show an error if addDirectory throws an exception', async () => { const error = new Error('Directory does not exist'); - const newPath = path.normalize('/home/user/invalid-project'); + const newPath = path.resolve('/home/user/invalid-project'); vi.mocked(mockWorkspaceContext.addDirectories).mockReturnValue({ added: [], failed: [{ path: newPath, error }], @@ -184,7 +194,7 @@ describe('directoryCommand', () => { it('should add directory directly when folder trust is disabled', async () => { if (!addCommand?.action) throw new Error('No action'); vi.spyOn(trustedFolders, 'isFolderTrustEnabled').mockReturnValue(false); - const newPath = path.normalize('/home/user/new-project'); + const newPath = path.resolve('/home/user/new-project'); vi.mocked(mockWorkspaceContext.addDirectories).mockReturnValue({ added: [newPath], failed: [], @@ -198,7 +208,7 @@ describe('directoryCommand', () => { }); it('should show an info message for an already added directory', async () => { - const existingPath = path.normalize('/home/user/project1'); + const existingPath = path.resolve('/home/user/project1'); if (!addCommand?.action) throw new Error('No action'); await addCommand.action(mockContext, existingPath); expect(mockContext.ui.addItem).toHaveBeenCalledWith( @@ -212,9 +222,33 @@ describe('directoryCommand', () => { ); }); + it('should show an info message for an already added directory specified as a relative path', async () => { + const existingPath = path.resolve('/home/user/project1'); + const relativePath = './project1'; + const absoluteRelativePath = path.resolve( + path.resolve('/test/dir'), + relativePath, + ); + + vi.mocked(fs.realpathSync).mockImplementation((p) => { + if (p === absoluteRelativePath) return existingPath; + return p as string; + }); + + if (!addCommand?.action) throw new Error('No action'); + await addCommand.action(mockContext, relativePath); + + expect(mockContext.ui.addItem).toHaveBeenCalledWith( + expect.objectContaining({ + type: MessageType.INFO, + text: `The following directories are already in the workspace:\n- ${relativePath}`, + }), + ); + }); + it('should handle a mix of successful and failed additions', async () => { - const validPath = path.normalize('/home/user/valid-project'); - const invalidPath = path.normalize('/home/user/invalid-project'); + const validPath = path.resolve('/home/user/valid-project'); + const invalidPath = path.resolve('/home/user/invalid-project'); const error = new Error('Directory does not exist'); vi.mocked(mockWorkspaceContext.addDirectories).mockReturnValue({ added: [validPath], @@ -318,7 +352,7 @@ describe('directoryCommand', () => { it('should add a trusted directory', async () => { if (!addCommand?.action) throw new Error('No action'); mockIsPathTrusted.mockReturnValue(true); - const newPath = path.normalize('/home/user/trusted-project'); + const newPath = path.resolve('/home/user/trusted-project'); vi.mocked(mockWorkspaceContext.addDirectories).mockReturnValue({ added: [newPath], failed: [], @@ -334,7 +368,7 @@ describe('directoryCommand', () => { it('should return a custom dialog for an explicitly untrusted directory (upgrade flow)', async () => { if (!addCommand?.action) throw new Error('No action'); mockIsPathTrusted.mockReturnValue(false); // DO_NOT_TRUST - const newPath = path.normalize('/home/user/untrusted-project'); + const newPath = path.resolve('/home/user/untrusted-project'); const result = await addCommand.action(mockContext, newPath); @@ -357,7 +391,7 @@ describe('directoryCommand', () => { it('should return a custom dialog for a directory with undefined trust', async () => { if (!addCommand?.action) throw new Error('No action'); mockIsPathTrusted.mockReturnValue(undefined); - const newPath = path.normalize('/home/user/undefined-trust-project'); + const newPath = path.resolve('/home/user/undefined-trust-project'); const result = await addCommand.action(mockContext, newPath); @@ -385,7 +419,7 @@ describe('directoryCommand', () => { source: 'file', }); mockIsPathTrusted.mockReturnValue(undefined); - const newPath = path.normalize('/home/user/new-project'); + const newPath = path.resolve('/home/user/new-project'); const result = await addCommand.action(mockContext, newPath); diff --git a/packages/cli/src/ui/commands/directoryCommand.tsx b/packages/cli/src/ui/commands/directoryCommand.tsx index be0a35a344..53ec7acb7f 100644 --- a/packages/cli/src/ui/commands/directoryCommand.tsx +++ b/packages/cli/src/ui/commands/directoryCommand.tsx @@ -20,6 +20,7 @@ import { } from '../utils/directoryUtils.js'; import type { Config } from '@google/gemini-cli-core'; import * as path from 'node:path'; +import * as fs from 'node:fs'; async function finishAddingDirectories( config: Config, @@ -100,7 +101,7 @@ export const directoryCommand: SlashCommand = { const workspaceContext = context.services.config.getWorkspaceContext(); const existingDirs = new Set( - workspaceContext.getDirectories().map((dir) => path.normalize(dir)), + workspaceContext.getDirectories().map((dir) => path.resolve(dir)), ); filteredSuggestions = suggestions.filter((s) => { @@ -172,12 +173,23 @@ export const directoryCommand: SlashCommand = { const pathsToProcess: string[] = []; for (const pathToAdd of pathsToAdd) { - const expandedPath = expandHomeDir(pathToAdd.trim()); - if (currentWorkspaceDirs.includes(expandedPath)) { - alreadyAdded.push(pathToAdd.trim()); - } else { - pathsToProcess.push(pathToAdd.trim()); + const trimmedPath = pathToAdd.trim(); + const expandedPath = expandHomeDir(trimmedPath); + try { + const absolutePath = path.resolve( + workspaceContext.targetDir, + expandedPath, + ); + const resolvedPath = fs.realpathSync(absolutePath); + if (currentWorkspaceDirs.includes(resolvedPath)) { + alreadyAdded.push(trimmedPath); + continue; + } + } catch (_e) { + // Path might not exist or be inaccessible. + // We'll let batchAddDirectories handle it later. } + pathsToProcess.push(trimmedPath); } if (alreadyAdded.length > 0) {