refactor(code-index): extract scan execution and harden orchestration

src/eslint-suppressions.json

@@ -1306,7 +1306,7 @@
 	},
 	"services/code-index/__tests__/orchestrator.spec.ts": {
 		"@typescript-eslint/no-explicit-any": {
-			"count": 25
+			"count": 23
 		}
 	},
 	"services/code-index/__tests__/service-factory.spec.ts": {
@@ -1389,11 +1389,6 @@
 			"count": 2
 		}
 	},
-	"services/code-index/orchestrator.ts": {
-		"@typescript-eslint/no-explicit-any": {
-			"count": 2
-		}
-	},
 	"services/code-index/processors/__tests__/file-watcher.spec.ts": {
 		"@typescript-eslint/no-explicit-any": {
 			"count": 25

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

@@ -0,0 +1,88 @@
+import { CodeIndexRecovery } from "../code-index-recovery"
+import { CodeIndexRun, type CodeIndexRunState } from "../code-index-run"
+import { StateHolder } from "../../../utils/StateHolder"
+
+vi.mock("@roo-code/telemetry", () => ({
+	TelemetryService: { instance: { captureEvent: vi.fn() } },
+}))
+vi.mock("../../../i18n", () => ({
+	t: (key: string, options?: { errorMessage?: unknown }) => `${key}:${options?.errorMessage ?? ""}`,
+}))
+
+function setup() {
+	const cache = { flush: vi.fn().mockResolvedValue(undefined), clearCacheFile: vi.fn().mockResolvedValue(undefined) }
+	const store = { clearCollection: vi.fn().mockResolvedValue(undefined) }
+	const state = { setSystemState: vi.fn() }
+	const watcher = { stop: vi.fn() }
+	const run = new CodeIndexRun(new AbortController(), new StateHolder<CodeIndexRunState>("running"))
+	return { cache, store, state, watcher, run, recovery: new CodeIndexRecovery(cache, store, state, watcher) }
+}
+
+describe("CodeIndexRecovery", () => {
+	it.each([true, undefined])(
+		"preserves preexisting or unknown data during full-scan recovery (%s)",
+		async (presence) => {
+			const { recovery, cache, store, run } = setup()
+			run.preexistingCodePoints = presence
+			run.markScanStarted("full")
+			await recovery.handle(new Error("scan failed"), run)
+			expect(store.clearCollection).not.toHaveBeenCalled()
+			expect(cache.clearCacheFile).not.toHaveBeenCalled()
+		},
+	)
+
+	it.each(["preparation", "incremental", "full"] as const)("applies cleanup policy for %s failures", async (mode) => {
+		const { recovery, cache, store, state, watcher, run } = setup()
+		run.preexistingCodePoints = false
+		if (mode !== "preparation") run.markScanStarted(mode)
+		await recovery.handle(new Error("scan failed"), run)
+		expect(store.clearCollection).toHaveBeenCalledTimes(mode === "full" ? 1 : 0)
+		expect(cache.clearCacheFile).toHaveBeenCalledTimes(mode === "full" ? 1 : 0)
+		expect(state.setSystemState).toHaveBeenLastCalledWith("Error", expect.stringContaining("scan failed"))
+		expect(watcher.stop).toHaveBeenCalledOnce()
+		expect(run.state.value).toBe("running")
+	})
+
+	it.each(["signal", "AbortError"])("preserves data when cancellation is identified by %s", async (kind) => {
+		const { recovery, cache, store, state, watcher, run } = setup()
+		run.markScanStarted("full")
+		if (kind === "signal") run.cancel()
+		const error = kind === "signal" ? new Error("interrupted") : new DOMException("Stopped", "AbortError")
+		await recovery.handle(error, run)
+		expect(cache.flush).toHaveBeenCalledOnce()
+		expect(store.clearCollection).not.toHaveBeenCalled()
+		expect(cache.clearCacheFile).not.toHaveBeenCalled()
+		expect(watcher.stop).toHaveBeenCalledOnce()
+		expect(state.setSystemState).toHaveBeenLastCalledWith("Standby", expect.any(String))
+		expect(run.state.value).not.toBe("finished")
+	})
+
+	it("finishes cancellation recovery even if flushing fails", async () => {
+		const { recovery, cache, state, watcher, run } = setup()
+		run.cancel()
+		cache.flush.mockRejectedValue(new Error("flush failed"))
+		await recovery.handle(new Error("stopped"), run)
+		expect(watcher.stop).toHaveBeenCalledOnce()
+		expect(state.setSystemState).toHaveBeenLastCalledWith("Standby", expect.any(String))
+	})
+
+	it("attempts cache cleanup after collection cleanup fails and preserves the original failure", async () => {
+		const { recovery, cache, store, state, watcher, run } = setup()
+		run.preexistingCodePoints = false
+		run.markScanStarted("full")
+		store.clearCollection.mockRejectedValue(new Error("collection cleanup failed"))
+		cache.clearCacheFile.mockRejectedValue(new Error("cache cleanup failed"))
+		await recovery.handle(new Error("original failure"), run)
+		expect(cache.clearCacheFile).toHaveBeenCalledOnce()
+		expect(watcher.stop).toHaveBeenCalledOnce()
+		expect(state.setSystemState).toHaveBeenLastCalledWith("Error", expect.stringContaining("original failure"))
+	})
+
+	it("reports clearing errors without initiating additional destructive cleanup", () => {
+		const { recovery, cache, store, state } = setup()
+		recovery.handleClearError("delete failed")
+		expect(state.setSystemState).toHaveBeenCalledWith("Error", "Failed to clear index data: delete failed")
+		expect(store.clearCollection).not.toHaveBeenCalled()
+		expect(cache.clearCacheFile).not.toHaveBeenCalled()
+	})
+})

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

@@ -0,0 +1,110 @@
+import { CodeIndexRun, CodeIndexRunState } from "../code-index-run"
+import { StateHolder } from "../../../utils/StateHolder"
+
+function createRun(): CodeIndexRun {
+	return new CodeIndexRun(new AbortController(), new StateHolder<CodeIndexRunState>("running"))
+}
+
+describe("CodeIndexRun", () => {
+	it("uses the injected controller and state holder", async () => {
+		const controller = new AbortController()
+		const stateHolder = new StateHolder<CodeIndexRunState>("running")
+		const run = new CodeIndexRun(controller, stateHolder)
+		expect(run.signal).toBe(controller.signal)
+		expect(run.state).toBe(stateHolder)
+		run.cancel()
+		expect(controller.signal.aborted).toBe(true)
+		expect(stateHolder.value).toBe("cancelling")
+		run.finish()
+		expect(stateHolder.value).toBe("finished")
+		await expect(run.waitUntilFinished()).resolves.toBeUndefined()
+	})
+
+	it("starts without cancellation or destructive cleanup eligibility", () => {
+		const run = createRun()
+		expect(run.signal.aborted).toBe(false)
+		expect(run.fullScanStarted).toBe(false)
+	})
+
+	it.each(["full", "incremental"] as const)("records when a %s scan starts", (mode) => {
+		const run = createRun()
+		run.markScanStarted(mode)
+		expect(run.fullScanStarted).toBe(mode === "full")
+	})
+
+	it("keeps completion pending after cancellation until cleanup finishes", async () => {
+		const run = createRun()
+		const finished = vi.fn()
+		const completion = run.waitUntilFinished().then(finished)
+		const aborted = vi.fn()
+		run.signal.addEventListener("abort", aborted)
+
+		run.cancel()
+		run.cancel()
+		await Promise.resolve()
+		expect(run.signal.aborted).toBe(true)
+		expect(aborted).toHaveBeenCalledOnce()
+		expect(finished).not.toHaveBeenCalled()
+
+		run.finish()
+		run.finish()
+		await completion
+		expect(finished).toHaveBeenCalledOnce()
+	})
+
+	it("finishes a successful run without cancelling it", async () => {
+		const run = createRun()
+		run.finish()
+		await expect(run.waitUntilFinished()).resolves.toBeUndefined()
+		expect(run.signal.aborted).toBe(false)
+	})
+
+	it("isolates cancellation, scan mode and completion between runs", async () => {
+		const first = createRun()
+		const second = createRun()
+		const secondFinished = vi.fn()
+		const completion = second.waitUntilFinished().then(secondFinished)
+		first.markScanStarted("full")
+		first.cancel()
+		first.finish()
+		await first.waitUntilFinished()
+
+		expect(second.signal.aborted).toBe(false)
+		expect(second.fullScanStarted).toBe(false)
+		expect(secondFinished).not.toHaveBeenCalled()
+		second.finish()
+		await completion
+	})
+
+	it("notifies all waiters and releases their subscriptions", async () => {
+		const run = createRun()
+		const firstNotified = vi.fn()
+		const secondNotified = vi.fn()
+		const first = run.waitUntilFinished().then(firstNotified)
+		const second = run.waitUntilFinished().then(secondNotified)
+		await Promise.resolve()
+		expect(firstNotified).not.toHaveBeenCalled()
+		expect(secondNotified).not.toHaveBeenCalled()
+
+		run.finish()
+		await Promise.all([first, second])
+		expect(firstNotified).toHaveBeenCalledOnce()
+		expect(secondNotified).toHaveBeenCalledOnce()
+
+		await expect(run.waitUntilFinished()).resolves.toBeUndefined()
+		expect(run.state.value).toBe("finished")
+	})
+
+	it("replays state and publishes cancellation followed by completion", () => {
+		const run = createRun()
+		const states: string[] = []
+		const subscription = run.state.subscribe((state) => states.push(state))
+		run.cancel()
+		run.cancel()
+		run.finish()
+		run.finish()
+		run.cancel()
+		expect(states).toEqual(["running", "cancelling", "finished"])
+		subscription.unsubscribe()
+	})
+})

src/services/code-index/__tests__/code-index-scan-executor.spec.ts

@@ -0,0 +1,79 @@
+import { CodeIndexScanExecutor } from "../code-index-scan-executor"
+import type { IDirectoryScanner } from "../interfaces"
+
+vi.mock("../../../i18n", () => ({ t: (key: string) => key }))
+
+describe("CodeIndexScanExecutor", () => {
+	function setup() {
+		const scanner = { scanDirectory: vi.fn<IDirectoryScanner["scanDirectory"]>() }
+		const vectorStore = { markIndexingIncomplete: vi.fn().mockResolvedValue(undefined) }
+		const stateManager = { setSystemState: vi.fn(), reportBlockIndexingProgress: vi.fn() }
+		const executor = new CodeIndexScanExecutor("/workspace", scanner, vectorStore, stateManager)
+		return { scanner, vectorStore, stateManager, executor }
+	}
+
+	it.each(["runFullScan", "runIncrementalScan"] as const)("%s rejects a missing scanner result", async (method) => {
+		const { executor } = setup()
+		// An unconfigured mock returns undefined, simulating a broken scanner contract.
+		await expect(executor[method](new AbortController().signal)).rejects.toThrow(
+			method === "runFullScan"
+				? "Scan failed, is scanner initialized?"
+				: "Incremental scan failed, is scanner initialized?",
+		)
+	})
+
+	it.each(["runFullScan", "runIncrementalScan"] as const)(
+		"%s reports progress without completing the operation",
+		async (method) => {
+			const { scanner, vectorStore, stateManager, executor } = setup()
+			const signal = new AbortController().signal
+			scanner.scanDirectory.mockImplementation(async (_path, _onError, onIndexed, onParsed, receivedSignal) => {
+				expect(receivedSignal).toBe(signal)
+				expect(vectorStore.markIndexingIncomplete).toHaveBeenCalledOnce()
+				onParsed?.(3)
+				onIndexed?.(1)
+				onIndexed?.(2)
+				return { stats: { processed: 1, skipped: 0 }, totalBlockCount: 3 }
+			})
+			await executor[method](signal)
+			expect(stateManager.reportBlockIndexingProgress.mock.calls).toEqual([
+				[0, 3],
+				[1, 3],
+				[3, 3],
+			])
+			expect(stateManager.setSystemState).not.toHaveBeenCalledWith("Indexed", expect.any(String))
+		},
+	)
+
+	it.each(["runFullScan", "runIncrementalScan"] as const)(
+		"%s propagates cancellation to its owner",
+		async (method) => {
+			const { scanner, executor } = setup()
+			const controller = new AbortController()
+			scanner.scanDirectory.mockImplementation(async () => {
+				controller.abort()
+				return { stats: { processed: 0, skipped: 0 }, totalBlockCount: 0 }
+			})
+			await expect(executor[method](controller.signal)).rejects.toMatchObject({ name: "AbortError" })
+		},
+	)
+
+	it.each([
+		{ method: "runFullScan", rejects: false },
+		{ method: "runIncrementalScan", rejects: true },
+	] as const)("$method preserves its partial-failure policy", async ({ method, rejects }) => {
+		const { scanner, executor } = setup()
+		scanner.scanDirectory.mockImplementation(async (_path, onError, onIndexed, onParsed) => {
+			onParsed?.(10)
+			onIndexed?.(9)
+			onError?.(new Error("batch failed"))
+			return { stats: { processed: 2, skipped: 0 }, totalBlockCount: 10 }
+		})
+		const result = executor[method](new AbortController().signal)
+		if (rejects) {
+			await expect(result).rejects.toThrow("batch failed")
+		} else {
+			await expect(result).resolves.toBeUndefined()
+		}
+	})
+})