Skip to content

Commit 65e1821

Browse files
committed
Fixes #4875: Fix codebase re-indexing race condition
- Fixed race condition in CodeIndexOrchestrator where clearIndexData and startIndexing could conflict - Separated _isProcessing check from state validation in startIndexing for better error reporting - Made webview startIndexing handler properly await the operation - Removed _isProcessing reset from stopWatcher to prevent interference with calling methods - Added comprehensive tests for clearIndexData and startIndexing sequence - Ensures users can successfully re-index after clearing index data
1 parent 2e2f83b commit 65e1821

File tree

3 files changed

+101
-10
lines changed

3 files changed

+101
-10
lines changed

src/core/webview/webviewMessageHandler.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1440,7 +1440,7 @@ export const webviewMessageHandler = async (
14401440
await manager.initialize(provider.contextProxy)
14411441
}
14421442

1443-
manager.startIndexing()
1443+
await manager.startIndexing()
14441444
}
14451445
} catch (error) {
14461446
provider.log(`Error starting indexing: ${error instanceof Error ? error.message : String(error)}`)

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

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,4 +115,89 @@ describe("CodeIndexManager - handleExternalSettingsChange regression", () => {
115115
await expect(manager.handleExternalSettingsChange()).resolves.not.toThrow()
116116
})
117117
})
118+
119+
describe("clearIndexData and startIndexing sequence", () => {
120+
it("should allow startIndexing immediately after clearIndexData completes", async () => {
121+
// Mock the required dependencies
122+
const mockConfigManager = {
123+
loadConfiguration: vitest.fn().mockResolvedValue({ requiresRestart: false }),
124+
isFeatureEnabled: true,
125+
isFeatureConfigured: true,
126+
}
127+
const mockOrchestrator = {
128+
clearIndexData: vitest.fn().mockResolvedValue(undefined),
129+
startIndexing: vitest.fn().mockResolvedValue(undefined),
130+
stopWatcher: vitest.fn(),
131+
}
132+
const mockCacheManager = {
133+
clearCacheFile: vitest.fn().mockResolvedValue(undefined),
134+
}
135+
136+
// Set up the manager with mocked dependencies
137+
;(manager as any)._configManager = mockConfigManager
138+
;(manager as any)._orchestrator = mockOrchestrator
139+
;(manager as any)._searchService = {}
140+
;(manager as any)._cacheManager = mockCacheManager
141+
142+
// Mock the feature state
143+
vitest.spyOn(manager, "isFeatureEnabled", "get").mockReturnValue(true)
144+
vitest.spyOn(manager, "isFeatureConfigured", "get").mockReturnValue(true)
145+
146+
// Verify manager is considered initialized
147+
expect(manager.isInitialized).toBe(true)
148+
149+
// Test the sequence: clearIndexData followed by startIndexing
150+
await manager.clearIndexData()
151+
expect(mockOrchestrator.clearIndexData).toHaveBeenCalled()
152+
expect(mockCacheManager.clearCacheFile).toHaveBeenCalled()
153+
154+
// This should not throw an error about being in processing state
155+
await expect(manager.startIndexing()).resolves.not.toThrow()
156+
expect(mockOrchestrator.startIndexing).toHaveBeenCalled()
157+
})
158+
159+
it("should handle rapid clearIndexData and startIndexing calls", async () => {
160+
// Mock the required dependencies
161+
const mockConfigManager = {
162+
loadConfiguration: vitest.fn().mockResolvedValue({ requiresRestart: false }),
163+
isFeatureEnabled: true,
164+
isFeatureConfigured: true,
165+
}
166+
const mockOrchestrator = {
167+
clearIndexData: vitest
168+
.fn()
169+
.mockImplementation(() => new Promise((resolve) => setTimeout(resolve, 100))),
170+
startIndexing: vitest.fn().mockResolvedValue(undefined),
171+
stopWatcher: vitest.fn(),
172+
}
173+
const mockCacheManager = {
174+
clearCacheFile: vitest.fn().mockResolvedValue(undefined),
175+
}
176+
177+
// Set up the manager with mocked dependencies
178+
;(manager as any)._configManager = mockConfigManager
179+
;(manager as any)._orchestrator = mockOrchestrator
180+
;(manager as any)._searchService = {}
181+
;(manager as any)._cacheManager = mockCacheManager
182+
183+
// Mock the feature state
184+
vitest.spyOn(manager, "isFeatureEnabled", "get").mockReturnValue(true)
185+
vitest.spyOn(manager, "isFeatureConfigured", "get").mockReturnValue(true)
186+
187+
// Test rapid sequence: start clearIndexData and immediately call startIndexing
188+
const clearPromise = manager.clearIndexData()
189+
190+
// Wait a bit to ensure clearIndexData has started but not finished
191+
await new Promise((resolve) => setTimeout(resolve, 50))
192+
193+
// This should wait for clearIndexData to complete before proceeding
194+
const startPromise = manager.startIndexing()
195+
196+
// Both should complete successfully
197+
await Promise.all([clearPromise, startPromise])
198+
199+
expect(mockOrchestrator.clearIndexData).toHaveBeenCalled()
200+
expect(mockOrchestrator.startIndexing).toHaveBeenCalled()
201+
})
202+
})
118203
})

src/services/code-index/orchestrator.ts

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -93,18 +93,22 @@ export class CodeIndexOrchestrator {
9393
return
9494
}
9595

96-
if (
97-
this._isProcessing ||
98-
(this.stateManager.state !== "Standby" &&
99-
this.stateManager.state !== "Error" &&
100-
this.stateManager.state !== "Indexed")
101-
) {
96+
if (this._isProcessing) {
10297
console.warn(
103-
`[CodeIndexOrchestrator] Start rejected: Already processing or in state ${this.stateManager.state}.`,
98+
`[CodeIndexOrchestrator] Start rejected: Already processing (state: ${this.stateManager.state}).`,
10499
)
105100
return
106101
}
107102

103+
if (
104+
this.stateManager.state !== "Standby" &&
105+
this.stateManager.state !== "Error" &&
106+
this.stateManager.state !== "Indexed"
107+
) {
108+
console.warn(`[CodeIndexOrchestrator] Start rejected: Invalid state ${this.stateManager.state}.`)
109+
return
110+
}
111+
108112
this._isProcessing = true
109113
this.stateManager.setSystemState("Indexing", "Initializing services...")
110114

@@ -179,7 +183,7 @@ export class CodeIndexOrchestrator {
179183
if (this.stateManager.state !== "Error") {
180184
this.stateManager.setSystemState("Standby", "File watcher stopped.")
181185
}
182-
this._isProcessing = false
186+
// Note: Don't reset _isProcessing here as it may be managed by calling methods
183187
}
184188

185189
/**
@@ -190,7 +194,8 @@ export class CodeIndexOrchestrator {
190194
this._isProcessing = true
191195

192196
try {
193-
await this.stopWatcher()
197+
// Stop the watcher first
198+
this.stopWatcher()
194199

195200
try {
196201
if (this.configManager.isFeatureConfigured) {
@@ -201,6 +206,7 @@ export class CodeIndexOrchestrator {
201206
} catch (error: any) {
202207
console.error("[CodeIndexOrchestrator] Failed to clear vector collection:", error)
203208
this.stateManager.setSystemState("Error", `Failed to clear vector collection: ${error.message}`)
209+
return // Exit early on error, _isProcessing will be reset in finally
204210
}
205211

206212
await this.cacheManager.clearCacheFile()

0 commit comments

Comments
 (0)