mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-07 08:26:51 +00:00
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
This commit is contained in:
parent
2e2f83be60
commit
65e18215a1
3 changed files with 101 additions and 10 deletions
|
|
@ -1440,7 +1440,7 @@ export const webviewMessageHandler = async (
|
|||
await manager.initialize(provider.contextProxy)
|
||||
}
|
||||
|
||||
manager.startIndexing()
|
||||
await manager.startIndexing()
|
||||
}
|
||||
} catch (error) {
|
||||
provider.log(`Error starting indexing: ${error instanceof Error ? error.message : String(error)}`)
|
||||
|
|
|
|||
|
|
@ -115,4 +115,89 @@ describe("CodeIndexManager - handleExternalSettingsChange regression", () => {
|
|||
await expect(manager.handleExternalSettingsChange()).resolves.not.toThrow()
|
||||
})
|
||||
})
|
||||
|
||||
describe("clearIndexData and startIndexing sequence", () => {
|
||||
it("should allow startIndexing immediately after clearIndexData completes", async () => {
|
||||
// Mock the required dependencies
|
||||
const mockConfigManager = {
|
||||
loadConfiguration: vitest.fn().mockResolvedValue({ requiresRestart: false }),
|
||||
isFeatureEnabled: true,
|
||||
isFeatureConfigured: true,
|
||||
}
|
||||
const mockOrchestrator = {
|
||||
clearIndexData: vitest.fn().mockResolvedValue(undefined),
|
||||
startIndexing: vitest.fn().mockResolvedValue(undefined),
|
||||
stopWatcher: vitest.fn(),
|
||||
}
|
||||
const mockCacheManager = {
|
||||
clearCacheFile: vitest.fn().mockResolvedValue(undefined),
|
||||
}
|
||||
|
||||
// Set up the manager with mocked dependencies
|
||||
;(manager as any)._configManager = mockConfigManager
|
||||
;(manager as any)._orchestrator = mockOrchestrator
|
||||
;(manager as any)._searchService = {}
|
||||
;(manager as any)._cacheManager = mockCacheManager
|
||||
|
||||
// Mock the feature state
|
||||
vitest.spyOn(manager, "isFeatureEnabled", "get").mockReturnValue(true)
|
||||
vitest.spyOn(manager, "isFeatureConfigured", "get").mockReturnValue(true)
|
||||
|
||||
// Verify manager is considered initialized
|
||||
expect(manager.isInitialized).toBe(true)
|
||||
|
||||
// Test the sequence: clearIndexData followed by startIndexing
|
||||
await manager.clearIndexData()
|
||||
expect(mockOrchestrator.clearIndexData).toHaveBeenCalled()
|
||||
expect(mockCacheManager.clearCacheFile).toHaveBeenCalled()
|
||||
|
||||
// This should not throw an error about being in processing state
|
||||
await expect(manager.startIndexing()).resolves.not.toThrow()
|
||||
expect(mockOrchestrator.startIndexing).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it("should handle rapid clearIndexData and startIndexing calls", async () => {
|
||||
// Mock the required dependencies
|
||||
const mockConfigManager = {
|
||||
loadConfiguration: vitest.fn().mockResolvedValue({ requiresRestart: false }),
|
||||
isFeatureEnabled: true,
|
||||
isFeatureConfigured: true,
|
||||
}
|
||||
const mockOrchestrator = {
|
||||
clearIndexData: vitest
|
||||
.fn()
|
||||
.mockImplementation(() => new Promise((resolve) => setTimeout(resolve, 100))),
|
||||
startIndexing: vitest.fn().mockResolvedValue(undefined),
|
||||
stopWatcher: vitest.fn(),
|
||||
}
|
||||
const mockCacheManager = {
|
||||
clearCacheFile: vitest.fn().mockResolvedValue(undefined),
|
||||
}
|
||||
|
||||
// Set up the manager with mocked dependencies
|
||||
;(manager as any)._configManager = mockConfigManager
|
||||
;(manager as any)._orchestrator = mockOrchestrator
|
||||
;(manager as any)._searchService = {}
|
||||
;(manager as any)._cacheManager = mockCacheManager
|
||||
|
||||
// Mock the feature state
|
||||
vitest.spyOn(manager, "isFeatureEnabled", "get").mockReturnValue(true)
|
||||
vitest.spyOn(manager, "isFeatureConfigured", "get").mockReturnValue(true)
|
||||
|
||||
// Test rapid sequence: start clearIndexData and immediately call startIndexing
|
||||
const clearPromise = manager.clearIndexData()
|
||||
|
||||
// Wait a bit to ensure clearIndexData has started but not finished
|
||||
await new Promise((resolve) => setTimeout(resolve, 50))
|
||||
|
||||
// This should wait for clearIndexData to complete before proceeding
|
||||
const startPromise = manager.startIndexing()
|
||||
|
||||
// Both should complete successfully
|
||||
await Promise.all([clearPromise, startPromise])
|
||||
|
||||
expect(mockOrchestrator.clearIndexData).toHaveBeenCalled()
|
||||
expect(mockOrchestrator.startIndexing).toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -93,18 +93,22 @@ export class CodeIndexOrchestrator {
|
|||
return
|
||||
}
|
||||
|
||||
if (
|
||||
this._isProcessing ||
|
||||
(this.stateManager.state !== "Standby" &&
|
||||
this.stateManager.state !== "Error" &&
|
||||
this.stateManager.state !== "Indexed")
|
||||
) {
|
||||
if (this._isProcessing) {
|
||||
console.warn(
|
||||
`[CodeIndexOrchestrator] Start rejected: Already processing or in state ${this.stateManager.state}.`,
|
||||
`[CodeIndexOrchestrator] Start rejected: Already processing (state: ${this.stateManager.state}).`,
|
||||
)
|
||||
return
|
||||
}
|
||||
|
||||
if (
|
||||
this.stateManager.state !== "Standby" &&
|
||||
this.stateManager.state !== "Error" &&
|
||||
this.stateManager.state !== "Indexed"
|
||||
) {
|
||||
console.warn(`[CodeIndexOrchestrator] Start rejected: Invalid state ${this.stateManager.state}.`)
|
||||
return
|
||||
}
|
||||
|
||||
this._isProcessing = true
|
||||
this.stateManager.setSystemState("Indexing", "Initializing services...")
|
||||
|
||||
|
|
@ -179,7 +183,7 @@ export class CodeIndexOrchestrator {
|
|||
if (this.stateManager.state !== "Error") {
|
||||
this.stateManager.setSystemState("Standby", "File watcher stopped.")
|
||||
}
|
||||
this._isProcessing = false
|
||||
// Note: Don't reset _isProcessing here as it may be managed by calling methods
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -190,7 +194,8 @@ export class CodeIndexOrchestrator {
|
|||
this._isProcessing = true
|
||||
|
||||
try {
|
||||
await this.stopWatcher()
|
||||
// Stop the watcher first
|
||||
this.stopWatcher()
|
||||
|
||||
try {
|
||||
if (this.configManager.isFeatureConfigured) {
|
||||
|
|
@ -201,6 +206,7 @@ export class CodeIndexOrchestrator {
|
|||
} catch (error: any) {
|
||||
console.error("[CodeIndexOrchestrator] Failed to clear vector collection:", error)
|
||||
this.stateManager.setSystemState("Error", `Failed to clear vector collection: ${error.message}`)
|
||||
return // Exit early on error, _isProcessing will be reset in finally
|
||||
}
|
||||
|
||||
await this.cacheManager.clearCacheFile()
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue