Skip to content

Commit 7d31966

Browse files
fix: resolve workspace path inconsistency in code indexing for multi-workspace scenarios (#4397) (#5403)
Co-authored-by: Daniel Riccio <[email protected]>
1 parent ad201cc commit 7d31966

File tree

7 files changed

+160
-46
lines changed

7 files changed

+160
-46
lines changed

src/services/code-index/__tests__/manager.spec.ts

Lines changed: 32 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,31 @@
1-
import { CodeIndexManager } from "../manager"
1+
// Mock vscode module
2+
vi.mock("vscode", () => ({
3+
workspace: {
4+
workspaceFolders: [
5+
{
6+
uri: { fsPath: "/test/workspace" },
7+
name: "test",
8+
index: 0,
9+
},
10+
],
11+
},
12+
}))
213

314
// Mock only the essential dependencies
4-
vitest.mock("../../../utils/path", () => ({
5-
getWorkspacePath: vitest.fn(() => "/test/workspace"),
15+
vi.mock("../../../utils/path", () => ({
16+
getWorkspacePath: vi.fn(() => "/test/workspace"),
617
}))
718

8-
vitest.mock("../state-manager", () => ({
9-
CodeIndexStateManager: vitest.fn().mockImplementation(() => ({
10-
onProgressUpdate: vitest.fn(),
11-
getCurrentStatus: vitest.fn(),
12-
dispose: vitest.fn(),
19+
vi.mock("../state-manager", () => ({
20+
CodeIndexStateManager: vi.fn().mockImplementation(() => ({
21+
onProgressUpdate: vi.fn(),
22+
getCurrentStatus: vi.fn(),
23+
dispose: vi.fn(),
1324
})),
1425
}))
1526

27+
import { CodeIndexManager } from "../manager"
28+
1629
describe("CodeIndexManager - handleSettingsChange regression", () => {
1730
let mockContext: any
1831
let manager: CodeIndexManager
@@ -27,7 +40,7 @@ describe("CodeIndexManager - handleSettingsChange regression", () => {
2740
globalState: {} as any,
2841
extensionUri: {} as any,
2942
extensionPath: "/test/extension",
30-
asAbsolutePath: vitest.fn(),
43+
asAbsolutePath: vi.fn(),
3144
storageUri: {} as any,
3245
storagePath: "/test/storage",
3346
globalStorageUri: {} as any,
@@ -58,13 +71,13 @@ describe("CodeIndexManager - handleSettingsChange regression", () => {
5871

5972
// Mock a minimal config manager that simulates first-time configuration
6073
const mockConfigManager = {
61-
loadConfiguration: vitest.fn().mockResolvedValue({ requiresRestart: true }),
74+
loadConfiguration: vi.fn().mockResolvedValue({ requiresRestart: true }),
6275
}
6376
;(manager as any)._configManager = mockConfigManager
6477

6578
// Mock the feature state to simulate valid configuration that would normally trigger restart
66-
vitest.spyOn(manager, "isFeatureEnabled", "get").mockReturnValue(true)
67-
vitest.spyOn(manager, "isFeatureConfigured", "get").mockReturnValue(true)
79+
vi.spyOn(manager, "isFeatureEnabled", "get").mockReturnValue(true)
80+
vi.spyOn(manager, "isFeatureConfigured", "get").mockReturnValue(true)
6881

6982
// The key test: this should NOT throw "CodeIndexManager not initialized" error
7083
await expect(manager.handleSettingsChange()).resolves.not.toThrow()
@@ -76,10 +89,10 @@ describe("CodeIndexManager - handleSettingsChange regression", () => {
7689
it("should work normally when manager is initialized", async () => {
7790
// Mock a complete config manager with all required properties
7891
const mockConfigManager = {
79-
loadConfiguration: vitest.fn().mockResolvedValue({ requiresRestart: true }),
92+
loadConfiguration: vi.fn().mockResolvedValue({ requiresRestart: true }),
8093
isFeatureConfigured: true,
8194
isFeatureEnabled: true,
82-
getConfig: vitest.fn().mockReturnValue({
95+
getConfig: vi.fn().mockReturnValue({
8396
isEnabled: true,
8497
isConfigured: true,
8598
embedderProvider: "openai",
@@ -93,20 +106,20 @@ describe("CodeIndexManager - handleSettingsChange regression", () => {
93106
;(manager as any)._configManager = mockConfigManager
94107

95108
// Simulate an initialized manager by setting the required properties
96-
;(manager as any)._orchestrator = { stopWatcher: vitest.fn() }
109+
;(manager as any)._orchestrator = { stopWatcher: vi.fn() }
97110
;(manager as any)._searchService = {}
98111
;(manager as any)._cacheManager = {}
99112

100113
// Verify manager is considered initialized
101114
expect(manager.isInitialized).toBe(true)
102115

103116
// Mock the methods that would be called during restart
104-
const recreateServicesSpy = vitest.spyOn(manager as any, "_recreateServices").mockImplementation(() => {})
105-
const startIndexingSpy = vitest.spyOn(manager, "startIndexing").mockResolvedValue()
117+
const recreateServicesSpy = vi.spyOn(manager as any, "_recreateServices").mockImplementation(() => {})
118+
const startIndexingSpy = vi.spyOn(manager, "startIndexing").mockResolvedValue()
106119

107120
// Mock the feature state
108-
vitest.spyOn(manager, "isFeatureEnabled", "get").mockReturnValue(true)
109-
vitest.spyOn(manager, "isFeatureConfigured", "get").mockReturnValue(true)
121+
vi.spyOn(manager, "isFeatureEnabled", "get").mockReturnValue(true)
122+
vi.spyOn(manager, "isFeatureConfigured", "get").mockReturnValue(true)
110123

111124
await manager.handleSettingsChange()
112125

src/services/code-index/manager.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,17 @@ export class CodeIndexManager {
2626
private _cacheManager: CacheManager | undefined
2727

2828
public static getInstance(context: vscode.ExtensionContext): CodeIndexManager | undefined {
29-
const workspacePath = getWorkspacePath() // Assumes single workspace for now
30-
31-
if (!workspacePath) {
29+
// Use first workspace folder consistently
30+
const workspaceFolders = vscode.workspace.workspaceFolders
31+
if (!workspaceFolders || workspaceFolders.length === 0) {
3232
return undefined
3333
}
3434

35+
// Always use the first workspace folder for consistency across all indexing operations.
36+
// This ensures that the same workspace context is used throughout the indexing pipeline,
37+
// preventing path resolution errors in multi-workspace scenarios.
38+
const workspacePath = workspaceFolders[0].uri.fsPath
39+
3540
if (!CodeIndexManager.instances.has(workspacePath)) {
3641
CodeIndexManager.instances.set(workspacePath, new CodeIndexManager(workspacePath, context))
3742
}

src/services/code-index/processors/file-watcher.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,7 @@ export class FileWatcher implements IFileWatcher {
464464
}
465465

466466
// Check if file should be ignored
467-
const relativeFilePath = generateRelativeFilePath(filePath)
467+
const relativeFilePath = generateRelativeFilePath(filePath, this.workspacePath)
468468
if (
469469
!this.ignoreController.validateAccess(filePath) ||
470470
(this.ignoreInstance && this.ignoreInstance.ignores(relativeFilePath))
@@ -512,15 +512,15 @@ export class FileWatcher implements IFileWatcher {
512512
const { embeddings } = await this.embedder.createEmbeddings(texts)
513513

514514
pointsToUpsert = blocks.map((block, index) => {
515-
const normalizedAbsolutePath = generateNormalizedAbsolutePath(block.file_path)
515+
const normalizedAbsolutePath = generateNormalizedAbsolutePath(block.file_path, this.workspacePath)
516516
const stableName = `${normalizedAbsolutePath}:${block.start_line}`
517517
const pointId = uuidv5(stableName, QDRANT_CODE_BLOCK_NAMESPACE)
518518

519519
return {
520520
id: pointId,
521521
vector: embeddings[index],
522522
payload: {
523-
filePath: generateRelativeFilePath(normalizedAbsolutePath),
523+
filePath: generateRelativeFilePath(normalizedAbsolutePath, this.workspacePath),
524524
codeChunk: block.content,
525525
startLine: block.start_line,
526526
endLine: block.end_line,

src/services/code-index/processors/scanner.ts

Lines changed: 34 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { RooIgnoreController } from "../../../core/ignore/RooIgnoreController"
44
import { stat } from "fs/promises"
55
import * as path from "path"
66
import { generateNormalizedAbsolutePath, generateRelativeFilePath } from "../shared/get-relative-path"
7+
import { getWorkspacePathForContext } from "../../../utils/path"
78
import { scannerExtensions } from "../shared/supported-extensions"
89
import * as vscode from "vscode"
910
import { CodeBlock, ICodeParser, IEmbedder, IVectorStore, IDirectoryScanner } from "../interfaces"
@@ -49,6 +50,9 @@ export class DirectoryScanner implements IDirectoryScanner {
4950
onFileParsed?: (fileBlockCount: number) => void,
5051
): Promise<{ codeBlocks: CodeBlock[]; stats: { processed: number; skipped: number }; totalBlockCount: number }> {
5152
const directoryPath = directory
53+
// Capture workspace context at scan start
54+
const scanWorkspace = getWorkspacePathForContext(directoryPath)
55+
5256
// Get all files recursively (handles .gitignore automatically)
5357
const [allPaths, _] = await listFiles(directoryPath, true, MAX_LIST_FILES_LIMIT)
5458

@@ -66,7 +70,7 @@ export class DirectoryScanner implements IDirectoryScanner {
6670
// Filter by supported extensions, ignore patterns, and excluded directories
6771
const supportedPaths = allowedPaths.filter((filePath) => {
6872
const ext = path.extname(filePath).toLowerCase()
69-
const relativeFilePath = generateRelativeFilePath(filePath)
73+
const relativeFilePath = generateRelativeFilePath(filePath, scanWorkspace)
7074

7175
// Check if file is in an ignored directory using the shared helper
7276
if (isPathInIgnoredDirectory(filePath)) {
@@ -169,6 +173,7 @@ export class DirectoryScanner implements IDirectoryScanner {
169173
batchBlocks,
170174
batchTexts,
171175
batchFileInfos,
176+
scanWorkspace,
172177
onError,
173178
onBlocksIndexed,
174179
),
@@ -185,12 +190,15 @@ export class DirectoryScanner implements IDirectoryScanner {
185190
await this.cacheManager.updateHash(filePath, currentFileHash)
186191
}
187192
} catch (error) {
188-
console.error(`Error processing file ${filePath}:`, error)
193+
console.error(`Error processing file ${filePath} in workspace ${scanWorkspace}:`, error)
189194
if (onError) {
190195
onError(
191196
error instanceof Error
192-
? error
193-
: new Error(t("embeddings:scanner.unknownErrorProcessingFile", { filePath })),
197+
? new Error(`${error.message} (Workspace: ${scanWorkspace}, File: ${filePath})`)
198+
: new Error(
199+
t("embeddings:scanner.unknownErrorProcessingFile", { filePath }) +
200+
` (Workspace: ${scanWorkspace})`,
201+
),
194202
)
195203
}
196204
}
@@ -214,7 +222,7 @@ export class DirectoryScanner implements IDirectoryScanner {
214222

215223
// Queue final batch processing
216224
const batchPromise = batchLimiter(() =>
217-
this.processBatch(batchBlocks, batchTexts, batchFileInfos, onError, onBlocksIndexed),
225+
this.processBatch(batchBlocks, batchTexts, batchFileInfos, scanWorkspace, onError, onBlocksIndexed),
218226
)
219227
activeBatchPromises.push(batchPromise)
220228
} finally {
@@ -235,15 +243,20 @@ export class DirectoryScanner implements IDirectoryScanner {
235243
await this.qdrantClient.deletePointsByFilePath(cachedFilePath)
236244
await this.cacheManager.deleteHash(cachedFilePath)
237245
} catch (error) {
238-
console.error(`[DirectoryScanner] Failed to delete points for ${cachedFilePath}:`, error)
246+
console.error(
247+
`[DirectoryScanner] Failed to delete points for ${cachedFilePath} in workspace ${scanWorkspace}:`,
248+
error,
249+
)
239250
if (onError) {
240251
onError(
241252
error instanceof Error
242-
? error
253+
? new Error(
254+
`${error.message} (Workspace: ${scanWorkspace}, File: ${cachedFilePath})`,
255+
)
243256
: new Error(
244257
t("embeddings:scanner.unknownErrorDeletingPoints", {
245258
filePath: cachedFilePath,
246-
}),
259+
}) + ` (Workspace: ${scanWorkspace})`,
247260
),
248261
)
249262
}
@@ -267,6 +280,7 @@ export class DirectoryScanner implements IDirectoryScanner {
267280
batchBlocks: CodeBlock[],
268281
batchTexts: string[],
269282
batchFileInfos: { filePath: string; fileHash: string; isNew: boolean }[],
283+
scanWorkspace: string,
270284
onError?: (error: Error) => void,
271285
onBlocksIndexed?: (indexedCount: number) => void,
272286
): Promise<void> {
@@ -292,11 +306,14 @@ export class DirectoryScanner implements IDirectoryScanner {
292306
await this.qdrantClient.deletePointsByMultipleFilePaths(uniqueFilePaths)
293307
} catch (deleteError) {
294308
console.error(
295-
`[DirectoryScanner] Failed to delete points for ${uniqueFilePaths.length} files before upsert:`,
309+
`[DirectoryScanner] Failed to delete points for ${uniqueFilePaths.length} files before upsert in workspace ${scanWorkspace}:`,
296310
deleteError,
297311
)
298-
// Re-throw the error to stop processing this batch attempt
299-
throw deleteError
312+
// Re-throw the error with workspace context
313+
throw new Error(
314+
`Failed to delete points for ${uniqueFilePaths.length} files. Workspace: ${scanWorkspace}. ${deleteError instanceof Error ? deleteError.message : String(deleteError)}`,
315+
{ cause: deleteError },
316+
)
300317
}
301318
}
302319
// --- End Deletion Step ---
@@ -306,7 +323,7 @@ export class DirectoryScanner implements IDirectoryScanner {
306323

307324
// Prepare points for Qdrant
308325
const points = batchBlocks.map((block, index) => {
309-
const normalizedAbsolutePath = generateNormalizedAbsolutePath(block.file_path)
326+
const normalizedAbsolutePath = generateNormalizedAbsolutePath(block.file_path, scanWorkspace)
310327

311328
// Use segmentHash for unique ID generation to handle multiple segments from same line
312329
const pointId = uuidv5(block.segmentHash, QDRANT_CODE_BLOCK_NAMESPACE)
@@ -315,7 +332,7 @@ export class DirectoryScanner implements IDirectoryScanner {
315332
id: pointId,
316333
vector: embeddings[index],
317334
payload: {
318-
filePath: generateRelativeFilePath(normalizedAbsolutePath),
335+
filePath: generateRelativeFilePath(normalizedAbsolutePath, scanWorkspace),
319336
codeChunk: block.content,
320337
startLine: block.start_line,
321338
endLine: block.end_line,
@@ -335,7 +352,10 @@ export class DirectoryScanner implements IDirectoryScanner {
335352
success = true
336353
} catch (error) {
337354
lastError = error as Error
338-
console.error(`[DirectoryScanner] Error processing batch (attempt ${attempts}):`, error)
355+
console.error(
356+
`[DirectoryScanner] Error processing batch (attempt ${attempts}) in workspace ${scanWorkspace}:`,
357+
error,
358+
)
339359

340360
if (attempts < MAX_BATCH_RETRIES) {
341361
const delay = INITIAL_RETRY_DELAY_MS * Math.pow(2, attempts - 1)
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
import { describe, it, expect } from "vitest"
2+
import path from "path"
3+
import { generateNormalizedAbsolutePath, generateRelativeFilePath } from "../get-relative-path"
4+
5+
describe("get-relative-path", () => {
6+
describe("generateNormalizedAbsolutePath", () => {
7+
it("should use provided workspace root", () => {
8+
const filePath = "src/file.ts"
9+
const workspaceRoot = path.join(path.sep, "custom", "workspace")
10+
const result = generateNormalizedAbsolutePath(filePath, workspaceRoot)
11+
// On Windows, path.resolve adds the drive letter, so we need to use path.resolve for the expected value
12+
expect(result).toBe(path.resolve(workspaceRoot, filePath))
13+
})
14+
15+
it("should handle absolute paths", () => {
16+
const filePath = path.join(path.sep, "absolute", "path", "file.ts")
17+
const workspaceRoot = path.join(path.sep, "custom", "workspace")
18+
const result = generateNormalizedAbsolutePath(filePath, workspaceRoot)
19+
// When an absolute path is provided, it should be resolved to include drive letter on Windows
20+
expect(result).toBe(path.resolve(filePath))
21+
})
22+
23+
it("should normalize paths with . and .. segments", () => {
24+
const filePath = "./src/../src/file.ts"
25+
const workspaceRoot = path.join(path.sep, "custom", "workspace")
26+
const result = generateNormalizedAbsolutePath(filePath, workspaceRoot)
27+
// Use path.resolve to get the expected normalized absolute path
28+
expect(result).toBe(path.resolve(workspaceRoot, "src", "file.ts"))
29+
})
30+
})
31+
32+
describe("generateRelativeFilePath", () => {
33+
it("should use provided workspace root", () => {
34+
const workspaceRoot = path.join(path.sep, "custom", "workspace")
35+
const absolutePath = path.join(workspaceRoot, "src", "file.ts")
36+
const result = generateRelativeFilePath(absolutePath, workspaceRoot)
37+
expect(result).toBe(path.join("src", "file.ts"))
38+
})
39+
40+
it("should handle paths outside workspace", () => {
41+
const absolutePath = path.join(path.sep, "outside", "workspace", "file.ts")
42+
const workspaceRoot = path.join(path.sep, "custom", "workspace")
43+
const result = generateRelativeFilePath(absolutePath, workspaceRoot)
44+
// The result will have .. segments to navigate outside
45+
expect(result).toContain("..")
46+
})
47+
48+
it("should handle same path as workspace", () => {
49+
const workspaceRoot = path.join(path.sep, "custom", "workspace")
50+
const absolutePath = workspaceRoot
51+
const result = generateRelativeFilePath(absolutePath, workspaceRoot)
52+
expect(result).toBe(".")
53+
})
54+
55+
it("should handle multi-workspace scenarios", () => {
56+
// Simulate the error scenario from the issue
57+
const workspaceRoot = path.join(path.sep, "Users", "test", "project")
58+
const absolutePath = path.join(path.sep, "Users", "test", "admin", ".prettierrc.json")
59+
const result = generateRelativeFilePath(absolutePath, workspaceRoot)
60+
// Should generate a valid relative path, not throw an error
61+
expect(result).toBe(path.join("..", "admin", ".prettierrc.json"))
62+
})
63+
})
64+
})

src/services/code-index/shared/get-relative-path.ts

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,14 @@
11
import path from "path"
2-
import { getWorkspacePath } from "../../../utils/path"
32

43
/**
54
* Generates a normalized absolute path from a given file path and workspace root.
65
* Handles path resolution and normalization to ensure consistent absolute paths.
76
*
87
* @param filePath - The file path to normalize (can be relative or absolute)
9-
* @param workspaceRoot - The root directory of the workspace
8+
* @param workspaceRoot - The root directory of the workspace (required)
109
* @returns The normalized absolute path
1110
*/
12-
export function generateNormalizedAbsolutePath(filePath: string): string {
13-
const workspaceRoot = getWorkspacePath()
11+
export function generateNormalizedAbsolutePath(filePath: string, workspaceRoot: string): string {
1412
// Resolve the path to make it absolute if it's relative
1513
const resolvedPath = path.resolve(workspaceRoot, filePath)
1614
// Normalize to handle any . or .. segments and duplicate slashes
@@ -22,11 +20,10 @@ export function generateNormalizedAbsolutePath(filePath: string): string {
2220
* Ensures consistent relative path generation across different platforms.
2321
*
2422
* @param normalizedAbsolutePath - The normalized absolute path to convert
25-
* @param workspaceRoot - The root directory of the workspace
23+
* @param workspaceRoot - The root directory of the workspace (required)
2624
* @returns The relative path from workspaceRoot to the file
2725
*/
28-
export function generateRelativeFilePath(normalizedAbsolutePath: string): string {
29-
const workspaceRoot = getWorkspacePath()
26+
export function generateRelativeFilePath(normalizedAbsolutePath: string, workspaceRoot: string): string {
3027
// Generate the relative path
3128
const relativePath = path.relative(workspaceRoot, normalizedAbsolutePath)
3229
// Normalize to ensure consistent path separators

0 commit comments

Comments
 (0)