From 5c5fc89eb16afb65a5bbcb30e3bc576ed55e66b8 Mon Sep 17 00:00:00 2001 From: christine betts Date: Thu, 14 Aug 2025 20:12:57 +0000 Subject: [PATCH] [ide-mode] Support multi-folder workspaces (#6177) --- packages/core/src/ide/ide-client.test.ts | 68 +++++++ packages/core/src/ide/ide-client.ts | 68 ++++--- .../core/src/utils/memoryImportProcessor.ts | 10 +- packages/core/src/utils/paths.test.ts | 106 +++++++++- packages/core/src/utils/paths.ts | 20 ++ .../src/extension-multi-folder.test.ts | 186 ++++++++++++++++++ .../vscode-ide-companion/src/extension.ts | 8 +- 7 files changed, 427 insertions(+), 39 deletions(-) create mode 100644 packages/core/src/ide/ide-client.test.ts create mode 100644 packages/vscode-ide-companion/src/extension-multi-folder.test.ts diff --git a/packages/core/src/ide/ide-client.test.ts b/packages/core/src/ide/ide-client.test.ts new file mode 100644 index 00000000..6955e495 --- /dev/null +++ b/packages/core/src/ide/ide-client.test.ts @@ -0,0 +1,68 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect } from 'vitest'; +import { IdeClient } from './ide-client.js'; + +describe('IdeClient.validateWorkspacePath', () => { + it('should return valid if cwd is a subpath of the IDE workspace path', () => { + const result = IdeClient.validateWorkspacePath( + '/Users/person/gemini-cli', + 'VS Code', + '/Users/person/gemini-cli/sub-dir', + ); + expect(result.isValid).toBe(true); + }); + + it('should return invalid if GEMINI_CLI_IDE_WORKSPACE_PATH is undefined', () => { + const result = IdeClient.validateWorkspacePath( + undefined, + 'VS Code', + '/Users/person/gemini-cli/sub-dir', + ); + expect(result.isValid).toBe(false); + expect(result.error).toContain('Failed to connect'); + }); + + it('should return invalid if GEMINI_CLI_IDE_WORKSPACE_PATH is empty', () => { + const result = IdeClient.validateWorkspacePath( + '', + 'VS Code', + '/Users/person/gemini-cli/sub-dir', + ); + expect(result.isValid).toBe(false); + expect(result.error).toContain('please open a workspace folder'); + }); + + it('should return invalid if cwd is not within the IDE workspace path', () => { + const result = IdeClient.validateWorkspacePath( + '/some/other/path', + 'VS Code', + '/Users/person/gemini-cli/sub-dir', + ); + expect(result.isValid).toBe(false); + expect(result.error).toContain('Directory mismatch'); + }); + + it('should handle multiple workspace paths and return valid', () => { + const result = IdeClient.validateWorkspacePath( + '/some/other/path:/Users/person/gemini-cli', + 'VS Code', + '/Users/person/gemini-cli/sub-dir', + ); + expect(result.isValid).toBe(true); + }); + + it('should return invalid if cwd is not in any of the multiple workspace paths', () => { + const result = IdeClient.validateWorkspacePath( + '/some/other/path:/another/path', + 'VS Code', + '/Users/person/gemini-cli/sub-dir', + ); + expect(result.isValid).toBe(false); + expect(result.error).toContain('Directory mismatch'); + }); +}); diff --git a/packages/core/src/ide/ide-client.ts b/packages/core/src/ide/ide-client.ts index 94107f21..810e82e0 100644 --- a/packages/core/src/ide/ide-client.ts +++ b/packages/core/src/ide/ide-client.ts @@ -5,6 +5,7 @@ */ import * as fs from 'node:fs'; +import { isSubpath } from '../utils/paths.js'; import { detectIde, DetectedIde, getIdeInfo } from '../ide/detect-ide.js'; import { ideContext, @@ -93,7 +94,14 @@ export class IdeClient { this.setState(IDEConnectionStatus.Connecting); - if (!this.validateWorkspacePath()) { + const { isValid, error } = IdeClient.validateWorkspacePath( + process.env['GEMINI_CLI_IDE_WORKSPACE_PATH'], + this.currentIdeDisplayName, + process.cwd(), + ); + + if (!isValid) { + this.setState(IDEConnectionStatus.Disconnected, error, true); return; } @@ -245,37 +253,41 @@ export class IdeClient { } } - private validateWorkspacePath(): boolean { - const ideWorkspacePath = process.env['GEMINI_CLI_IDE_WORKSPACE_PATH']; + static validateWorkspacePath( + ideWorkspacePath: string | undefined, + currentIdeDisplayName: string | undefined, + cwd: string, + ): { isValid: boolean; error?: string } { if (ideWorkspacePath === undefined) { - this.setState( - IDEConnectionStatus.Disconnected, - `Failed to connect to IDE companion extension for ${this.currentIdeDisplayName}. Please ensure the extension is running and try refreshing your terminal. To install the extension, run /ide install.`, - true, - ); - return false; - } - if (ideWorkspacePath === '') { - this.setState( - IDEConnectionStatus.Disconnected, - `To use this feature, please open a single workspace folder in ${this.currentIdeDisplayName} and try again.`, - true, - ); - return false; + return { + isValid: false, + error: `Failed to connect to IDE companion extension for ${currentIdeDisplayName}. Please ensure the extension is running and try refreshing your terminal. To install the extension, run /ide install.`, + }; } - const idePath = getRealPath(ideWorkspacePath).toLocaleLowerCase(); - const cwd = getRealPath(process.cwd()).toLocaleLowerCase(); - const rel = path.relative(idePath, cwd); - if (rel.startsWith('..') || path.isAbsolute(rel)) { - this.setState( - IDEConnectionStatus.Disconnected, - `Directory mismatch. Gemini CLI is running in a different location than the open workspace in ${this.currentIdeDisplayName}. Please run the CLI from the same directory as your project's root folder.`, - true, - ); - return false; + if (ideWorkspacePath === '') { + return { + isValid: false, + error: `To use this feature, please open a workspace folder in ${currentIdeDisplayName} and try again.`, + }; } - return true; + + const ideWorkspacePaths = ideWorkspacePath.split(':'); + const realCwd = getRealPath(cwd); + const isWithinWorkspace = ideWorkspacePaths.some((workspacePath) => { + const idePath = getRealPath(workspacePath); + return isSubpath(idePath, realCwd); + }); + + if (!isWithinWorkspace) { + return { + isValid: false, + error: `Directory mismatch. Gemini CLI is running in a different location than the open workspace in ${currentIdeDisplayName}. Please run the CLI from one of the following directories: ${ideWorkspacePaths.join( + ', ', + )}`, + }; + } + return { isValid: true }; } private getPortFromEnv(): string | undefined { diff --git a/packages/core/src/utils/memoryImportProcessor.ts b/packages/core/src/utils/memoryImportProcessor.ts index 751e0ace..c89b983b 100644 --- a/packages/core/src/utils/memoryImportProcessor.ts +++ b/packages/core/src/utils/memoryImportProcessor.ts @@ -6,6 +6,7 @@ import * as fs from 'fs/promises'; import * as path from 'path'; +import { isSubpath } from './paths.js'; import { marked } from 'marked'; // Simple console logger for import processing @@ -411,10 +412,7 @@ export function validateImportPath( const resolvedPath = path.resolve(basePath, importPath); - return allowedDirectories.some((allowedDir) => { - const normalizedAllowedDir = path.resolve(allowedDir); - const isSamePath = resolvedPath === normalizedAllowedDir; - const isSubPath = resolvedPath.startsWith(normalizedAllowedDir + path.sep); - return isSamePath || isSubPath; - }); + return allowedDirectories.some((allowedDir) => + isSubpath(allowedDir, resolvedPath), + ); } diff --git a/packages/core/src/utils/paths.test.ts b/packages/core/src/utils/paths.test.ts index d688c072..0e964672 100644 --- a/packages/core/src/utils/paths.test.ts +++ b/packages/core/src/utils/paths.test.ts @@ -4,8 +4,8 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, it, expect } from 'vitest'; -import { escapePath, unescapePath } from './paths.js'; +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { escapePath, unescapePath, isSubpath } from './paths.js'; describe('escapePath', () => { it('should escape spaces', () => { @@ -212,3 +212,105 @@ describe('unescapePath', () => { expect(unescapePath('file\\\\\\(test\\).txt')).toBe('file\\\\(test).txt'); }); }); + +describe('isSubpath', () => { + it('should return true for a direct subpath', () => { + expect(isSubpath('/a/b', '/a/b/c')).toBe(true); + }); + + it('should return true for the same path', () => { + expect(isSubpath('/a/b', '/a/b')).toBe(true); + }); + + it('should return false for a parent path', () => { + expect(isSubpath('/a/b/c', '/a/b')).toBe(false); + }); + + it('should return false for a completely different path', () => { + expect(isSubpath('/a/b', '/x/y')).toBe(false); + }); + + it('should handle relative paths', () => { + expect(isSubpath('a/b', 'a/b/c')).toBe(true); + expect(isSubpath('a/b', 'a/c')).toBe(false); + }); + + it('should handle paths with ..', () => { + expect(isSubpath('/a/b', '/a/b/../b/c')).toBe(true); + expect(isSubpath('/a/b', '/a/c/../b')).toBe(true); + }); + + it('should handle root paths', () => { + expect(isSubpath('/', '/a')).toBe(true); + expect(isSubpath('/a', '/')).toBe(false); + }); + + it('should handle trailing slashes', () => { + expect(isSubpath('/a/b/', '/a/b/c')).toBe(true); + expect(isSubpath('/a/b', '/a/b/c/')).toBe(true); + expect(isSubpath('/a/b/', '/a/b/c/')).toBe(true); + }); +}); + +describe('isSubpath on Windows', () => { + const originalPlatform = process.platform; + + beforeAll(() => { + Object.defineProperty(process, 'platform', { + value: 'win32', + }); + }); + + afterAll(() => { + Object.defineProperty(process, 'platform', { + value: originalPlatform, + }); + }); + + it('should return true for a direct subpath on Windows', () => { + expect(isSubpath('C:\\Users\\Test', 'C:\\Users\\Test\\file.txt')).toBe( + true, + ); + }); + + it('should return true for the same path on Windows', () => { + expect(isSubpath('C:\\Users\\Test', 'C:\\Users\\Test')).toBe(true); + }); + + it('should return false for a parent path on Windows', () => { + expect(isSubpath('C:\\Users\\Test\\file.txt', 'C:\\Users\\Test')).toBe( + false, + ); + }); + + it('should return false for a different drive on Windows', () => { + expect(isSubpath('C:\\Users\\Test', 'D:\\Users\\Test')).toBe(false); + }); + + it('should be case-insensitive for drive letters on Windows', () => { + expect(isSubpath('c:\\Users\\Test', 'C:\\Users\\Test\\file.txt')).toBe( + true, + ); + }); + + it('should be case-insensitive for path components on Windows', () => { + expect(isSubpath('C:\\Users\\Test', 'c:\\users\\test\\file.txt')).toBe( + true, + ); + }); + + it('should handle mixed slashes on Windows', () => { + expect(isSubpath('C:/Users/Test', 'C:\\Users\\Test\\file.txt')).toBe(true); + }); + + it('should handle trailing slashes on Windows', () => { + expect(isSubpath('C:\\Users\\Test\\', 'C:\\Users\\Test\\file.txt')).toBe( + true, + ); + }); + + it('should handle relative paths correctly on Windows', () => { + expect(isSubpath('Users\\Test', 'Users\\Test\\file.txt')).toBe(true); + expect(isSubpath('Users\\Test\\file.txt', 'Users\\Test')).toBe(false); + }); +}); diff --git a/packages/core/src/utils/paths.ts b/packages/core/src/utils/paths.ts index 7bab888e..e7cf54cc 100644 --- a/packages/core/src/utils/paths.ts +++ b/packages/core/src/utils/paths.ts @@ -200,3 +200,23 @@ export function getUserCommandsDir(): string { export function getProjectCommandsDir(projectRoot: string): string { return path.join(projectRoot, GEMINI_DIR, COMMANDS_DIR_NAME); } + +/** + * Checks if a path is a subpath of another path. + * @param parentPath The parent path. + * @param childPath The child path. + * @returns True if childPath is a subpath of parentPath, false otherwise. + */ +export function isSubpath(parentPath: string, childPath: string): boolean { + const isWindows = os.platform() === 'win32'; + const pathModule = isWindows ? path.win32 : path; + + // On Windows, path.relative is case-insensitive. On POSIX, it's case-sensitive. + const relative = pathModule.relative(parentPath, childPath); + + return ( + !relative.startsWith(`..${pathModule.sep}`) && + relative !== '..' && + !pathModule.isAbsolute(relative) + ); +} diff --git a/packages/vscode-ide-companion/src/extension-multi-folder.test.ts b/packages/vscode-ide-companion/src/extension-multi-folder.test.ts new file mode 100644 index 00000000..bba6ea99 --- /dev/null +++ b/packages/vscode-ide-companion/src/extension-multi-folder.test.ts @@ -0,0 +1,186 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import * as vscode from 'vscode'; +import { activate } from './extension.js'; + +vi.mock('vscode', () => ({ + window: { + createOutputChannel: vi.fn(() => ({ + appendLine: vi.fn(), + })), + showInformationMessage: vi.fn(), + createTerminal: vi.fn(() => ({ + show: vi.fn(), + sendText: vi.fn(), + })), + onDidChangeActiveTextEditor: vi.fn(), + activeTextEditor: undefined, + tabGroups: { + all: [], + close: vi.fn(), + }, + showTextDocument: vi.fn(), + }, + workspace: { + workspaceFolders: [], + onDidCloseTextDocument: vi.fn(), + registerTextDocumentContentProvider: vi.fn(), + onDidChangeWorkspaceFolders: vi.fn(), + }, + commands: { + registerCommand: vi.fn(), + executeCommand: vi.fn(), + }, + Uri: { + joinPath: vi.fn(), + file: (path: string) => ({ fsPath: path }), + }, + ExtensionMode: { + Development: 1, + Production: 2, + }, + EventEmitter: vi.fn(() => ({ + event: vi.fn(), + fire: vi.fn(), + dispose: vi.fn(), + })), +})); + +describe('activate with multiple folders', () => { + let context: vscode.ExtensionContext; + let onDidChangeWorkspaceFoldersCallback: ( + e: vscode.WorkspaceFoldersChangeEvent, + ) => void; + + beforeEach(() => { + context = { + subscriptions: [], + environmentVariableCollection: { + replace: vi.fn(), + }, + globalState: { + get: vi.fn().mockReturnValue(true), + update: vi.fn(), + }, + extensionUri: { + fsPath: '/path/to/extension', + }, + } as unknown as vscode.ExtensionContext; + + vi.mocked(vscode.workspace.onDidChangeWorkspaceFolders).mockImplementation( + (callback) => { + onDidChangeWorkspaceFoldersCallback = callback; + return { dispose: vi.fn() }; + }, + ); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('should set a single folder path', async () => { + const workspaceFoldersSpy = vi.spyOn( + vscode.workspace, + 'workspaceFolders', + 'get', + ); + workspaceFoldersSpy.mockReturnValue([ + { uri: { fsPath: '/foo/bar' } }, + ] as vscode.WorkspaceFolder[]); + + await activate(context); + + expect(context.environmentVariableCollection.replace).toHaveBeenCalledWith( + 'GEMINI_CLI_IDE_WORKSPACE_PATH', + '/foo/bar', + ); + }); + + it('should set multiple folder paths, separated by a colon', async () => { + const workspaceFoldersSpy = vi.spyOn( + vscode.workspace, + 'workspaceFolders', + 'get', + ); + workspaceFoldersSpy.mockReturnValue([ + { uri: { fsPath: '/foo/bar' } }, + { uri: { fsPath: '/baz/qux' } }, + ] as vscode.WorkspaceFolder[]); + + await activate(context); + + expect(context.environmentVariableCollection.replace).toHaveBeenCalledWith( + 'GEMINI_CLI_IDE_WORKSPACE_PATH', + '/foo/bar:/baz/qux', + ); + }); + + it('should set an empty string if no folders are open', async () => { + const workspaceFoldersSpy = vi.spyOn( + vscode.workspace, + 'workspaceFolders', + 'get', + ); + workspaceFoldersSpy.mockReturnValue([]); + + await activate(context); + + expect(context.environmentVariableCollection.replace).toHaveBeenCalledWith( + 'GEMINI_CLI_IDE_WORKSPACE_PATH', + '', + ); + }); + + it('should update the path when workspace folders change', async () => { + const workspaceFoldersSpy = vi.spyOn( + vscode.workspace, + 'workspaceFolders', + 'get', + ); + workspaceFoldersSpy.mockReturnValue([ + { uri: { fsPath: '/foo/bar' } }, + ] as vscode.WorkspaceFolder[]); + + await activate(context); + + expect(context.environmentVariableCollection.replace).toHaveBeenCalledWith( + 'GEMINI_CLI_IDE_WORKSPACE_PATH', + '/foo/bar', + ); + + // Simulate adding a folder + workspaceFoldersSpy.mockReturnValue([ + { uri: { fsPath: '/foo/bar' } }, + { uri: { fsPath: '/baz/qux' } }, + ] as vscode.WorkspaceFolder[]); + onDidChangeWorkspaceFoldersCallback({ + added: [{ uri: { fsPath: '/baz/qux' } } as vscode.WorkspaceFolder], + removed: [], + }); + + expect(context.environmentVariableCollection.replace).toHaveBeenCalledWith( + 'GEMINI_CLI_IDE_WORKSPACE_PATH', + '/foo/bar:/baz/qux', + ); + + // Simulate removing a folder + workspaceFoldersSpy.mockReturnValue([ + { uri: { fsPath: '/baz/qux' } }, + ] as vscode.WorkspaceFolder[]); + onDidChangeWorkspaceFoldersCallback({ + added: [], + removed: [{ uri: { fsPath: '/foo/bar' } } as vscode.WorkspaceFolder], + }); + + expect(context.environmentVariableCollection.replace).toHaveBeenCalledWith( + 'GEMINI_CLI_IDE_WORKSPACE_PATH', + '/baz/qux', + ); + }); +}); diff --git a/packages/vscode-ide-companion/src/extension.ts b/packages/vscode-ide-companion/src/extension.ts index 18217140..10aa41f8 100644 --- a/packages/vscode-ide-companion/src/extension.ts +++ b/packages/vscode-ide-companion/src/extension.ts @@ -20,11 +20,13 @@ let log: (message: string) => void = () => {}; function updateWorkspacePath(context: vscode.ExtensionContext) { const workspaceFolders = vscode.workspace.workspaceFolders; - if (workspaceFolders && workspaceFolders.length === 1) { - const workspaceFolder = workspaceFolders[0]; + if (workspaceFolders && workspaceFolders.length > 0) { + const workspacePaths = workspaceFolders + .map((folder) => folder.uri.fsPath) + .join(':'); context.environmentVariableCollection.replace( IDE_WORKSPACE_PATH_ENV_VAR, - workspaceFolder.uri.fsPath, + workspacePaths, ); } else { context.environmentVariableCollection.replace(