diff --git a/.changeset/single-writer-codebase-index.md b/.changeset/single-writer-codebase-index.md new file mode 100644 index 000000000000..e8a4bd5db9ab --- /dev/null +++ b/.changeset/single-writer-codebase-index.md @@ -0,0 +1,6 @@ +--- +"@kilocode/kilo-indexing": minor +--- + +Keep codebase indexing intact when several sessions share a workspace. Only one session now writes a workspace's index, and a session that finds a partially built index no longer discards it and starts over. + diff --git a/packages/kilo-indexing/src/indexing/cache-manager.ts b/packages/kilo-indexing/src/indexing/cache-manager.ts index ba5cd3735f17..6245264d3656 100644 --- a/packages/kilo-indexing/src/indexing/cache-manager.ts +++ b/packages/kilo-indexing/src/indexing/cache-manager.ts @@ -19,11 +19,11 @@ export class CacheManager implements ICacheManager { private saveTask = Promise.resolve() constructor( - private readonly cacheDirectory: string, + public readonly dir: string, private readonly workspacePath: string, ) { const hash = createHash("sha256").update(workspacePath).digest("hex") - this.cachePath = path.join(cacheDirectory, `roo-index-cache-${hash}.json`) + this.cachePath = path.join(dir, `roo-index-cache-${hash}.json`) } async initialize(): Promise { diff --git a/packages/kilo-indexing/src/indexing/interfaces/vector-store.ts b/packages/kilo-indexing/src/indexing/interfaces/vector-store.ts index 7712502f14a3..16df83caba35 100644 --- a/packages/kilo-indexing/src/indexing/interfaces/vector-store.ts +++ b/packages/kilo-indexing/src/indexing/interfaces/vector-store.ts @@ -70,10 +70,13 @@ export interface IVectorStore { collectionExists(): Promise /** - * Checks if the collection exists and has indexed points - * @returns Promise resolving to boolean indicating if the collection exists and has points + * Reports how far along the store is. `partial` means points exist but no + * successful scan finished, which is what a store owned by another live + * session looks like. Callers must treat `partial` as usable-but-untrusted + * and must never clear it on behalf of a session that does not own the + * writer lock. */ - hasIndexedData(): Promise + state(): Promise /** * Marks the indexing process as complete by storing metadata @@ -94,6 +97,13 @@ export interface VectorStoreSearchResult { payload?: Payload | null } +export type StoreStatus = "empty" | "partial" | "complete" + +export type StoreState = { + status: StoreStatus + points?: number +} + export interface Payload { filePath: string fileHash?: string diff --git a/packages/kilo-indexing/src/indexing/manager.ts b/packages/kilo-indexing/src/indexing/manager.ts index dcaed039b19c..05f8fdfc49ff 100644 --- a/packages/kilo-indexing/src/indexing/manager.ts +++ b/packages/kilo-indexing/src/indexing/manager.ts @@ -160,6 +160,13 @@ export class CodeIndexManager { if (!this.isFeatureEnabled || !this.isFeatureConfigured) return if (this._retryTask || this._isRecoveringFromError) return + // Lock contention is not failure: another live session holds the writer + // lock, so retrying with backoff only spins while the holder watches. + // The second session retries on its own triggers (settings, restart, + // watcher events); a retry here would also recreate services, tearing + // down the loser's search path for no gain. + if (event.error.includes("Another session is indexing this workspace")) return + if (this._retryAttempt >= this._retryMaxAttempts) { log.warn("indexing recovery retries exhausted", { workspacePath: this.workspacePath, diff --git a/packages/kilo-indexing/src/indexing/orchestrator.ts b/packages/kilo-indexing/src/indexing/orchestrator.ts index 9f4d376ecad4..e52fc3958050 100644 --- a/packages/kilo-indexing/src/indexing/orchestrator.ts +++ b/packages/kilo-indexing/src/indexing/orchestrator.ts @@ -11,6 +11,7 @@ import type { IndexingTelemetryTrigger, } from "./interfaces/telemetry" import type { IVectorStore } from "./interfaces/vector-store" +import type { StoreState } from "./interfaces" import { DirectoryScanner } from "./processors" import type { CacheManager } from "./cache-manager" import type { Disposable } from "./runtime" @@ -18,14 +19,22 @@ import { Log } from "../util/log" import { sanitizeErrorMessage } from "./shared/validation-helpers" import { DEFAULT_VECTOR_STORE } from "./constants" import type { WorktreeOverlay } from "./worktree-overlay" +import { acquire, type Writer } from "./writer-lock" const log = Log.create({ service: "indexing-orchestrator" }) +const EMPTY: StoreState = { status: "empty" } + export class CodeIndexOrchestrator { private _fileWatcherSubscriptions: Disposable[] = [] private _isProcessing = false private _cancelRequested = false private _active?: Promise + // The session's writer lock, held from the first successful run until + // shutdown (or release on error/clear). The file watcher keeps writing the + // store after every scan, so releasing per-run would let a second session + // acquire the lock and scan concurrently — the corruption the lock targets. + private _writer?: Writer constructor( private readonly configManager: CodeIndexConfigManager, @@ -97,6 +106,11 @@ export class CodeIndexOrchestrator { workspacePath: this.workspacePath, filesInBatch: paths.length, }) + // The watcher writes the store on its own after the scan, outside + // runIndexing. Re-check ownership before every batch: a heartbeat can + // lapse (event loop stall) long enough for another session to reclaim + // the lock, and this session must not keep committing without it. + void this._verifyWriter() if (this.stateManager.state !== "Indexing") { this.stateManager.setSystemState("Indexing", "Processing file changes...") } @@ -145,6 +159,68 @@ export class CodeIndexOrchestrator { return task } + /** + * Only one session may write a workspace's index. Without this, a second + * session starts a scan against a store the first one is still filling, + * clears it as unusable, and leaves both sessions committing to the same + * table. The lock is session-held (see _writer): once acquired, later runs + * reuse it instead of contending with themselves. + */ + private async _acquire(): Promise { + if (this._writer) { + // The session caches its writer across runs. Verify ownership before + // reusing it: a heartbeat that saw another session's token stops beating, + // but only a fresh check here stops this orchestrator from writing the + // store after it has lost the lock. + if (await this._writer.owned()) return this._writer + log.warn("lost the writer lock since the last run; reacquiring", { + workspacePath: this.workspacePath, + }) + this._writer = undefined + } + const dir = this.cacheManager.dir + if (!dir) { + log.warn("no cache directory for the writer lock, proceeding unlocked", { + workspacePath: this.workspacePath, + }) + return { release: async () => {}, owned: async () => true } + } + const writer = await acquire(dir, this.workspacePath) + if (writer) { + this._writer = writer + return writer + } + log.info("writer lock is held elsewhere", { workspacePath: this.workspacePath }) + return undefined + } + + private async _releaseWriter(): Promise { + const writer = this._writer + this._writer = undefined + await writer?.release() + } + + /** + * Stops the watcher when this session no longer owns the writer lock. + * + * The heartbeat stops beating after a takeover, but the watcher keeps + * receiving file events and writing the store. Checking once per batch + * bounds how much can be written after ownership is lost; shutting the + * watcher down turns a silent split-brain into a visible Error state. + */ + private async _verifyWriter(): Promise { + const writer = this._writer + if (!writer) return + if (await writer.owned()) return + if (this._writer !== writer) return + this._writer = undefined + log.error("writer lock lost; stopping the file watcher to avoid unlocked writes", { + workspacePath: this.workspacePath, + }) + this.stateManager.setSystemState("Error", "Lost the indexing writer lock to another session. File watching stopped.") + this.stopWatcher() + } + private async runIndexing(trigger: IndexingTelemetryTrigger): Promise { log.info("indexing start requested", { workspacePath: this.workspacePath, @@ -176,6 +252,7 @@ export class CodeIndexOrchestrator { } this._cancelRequested = false + this._isProcessing = true this.stateManager.setSystemState("Indexing", "Initializing services...") @@ -184,6 +261,27 @@ export class CodeIndexOrchestrator { let mode: IndexingTelemetryMode | undefined try { + // Acquired inside the try and session-held (see _writer): a throw between + // acquire and the body (e.g. from state setup below) releases below and + // must not leave the lock fresh for STALE_MS, blocking other sessions. + // The lock is NOT released on success: the file watcher keeps writing the + // store after the scan, so it stays held until shutdown or clearIndexData. + const writer = await this._acquire() + if (!writer) { + // Contention is worth observing, not a silent park: the manager + // surfaces start events as activity, so emit an error event the manager + // can retry or display rather than leaving the session in Standby with + // no record of the missed run. + this.emitError( + "orchestrator:startIndexing", + new Error("Another session is indexing this workspace."), + "scan", + trigger, + ) + this.stateManager.setSystemState("Standby", "Another session is indexing this workspace.") + return + } + this.overlay?.prepare() await this._startWatcher() @@ -214,8 +312,11 @@ export class CodeIndexOrchestrator { }) } - const hasExistingData = this.overlay ? false : await this.vectorStore.hasIndexedData() - if (!this.overlay && !hasExistingData) { + const index = this.overlay ? EMPTY : await this.vectorStore.state() + // Only an empty store is safe to clear. A partial one belongs to a run + // that died or is still going, and clearing it throws away every point + // it managed to embed. + if (!this.overlay && index.status === "empty") { if (!collectionCreated) await this.vectorStore.clearCollection() await this.cacheManager.clearCacheFile() log.info("cleared indexing cache before full scan", { @@ -225,7 +326,7 @@ export class CodeIndexOrchestrator { } log.info("checked vector store indexed data", { workspacePath: this.workspacePath, - hasExistingData, + status: index.status, collectionCreated, }) @@ -234,7 +335,7 @@ export class CodeIndexOrchestrator { return } - mode = hasExistingData && !collectionCreated ? "incremental" : "full" + mode = index.status === "complete" && !collectionCreated ? "incremental" : "full" if (mode === "incremental") { log.info("collection has existing data, running incremental scan") @@ -245,7 +346,7 @@ export class CodeIndexOrchestrator { } else { log.info("running full scan", { workspacePath: this.workspacePath, - hasExistingData, + status: index.status, collectionCreated, }) this.stateManager.setSystemState("Indexing", "Services ready. Starting workspace scan...") @@ -265,6 +366,9 @@ export class CodeIndexOrchestrator { const msg = err instanceof Error ? err.message : "Unknown error" this.stateManager.setSystemState("Error", `Failed during initial scan: ${msg}`) this.stopWatcher() + // The watcher is stopped, so this session no longer writes the store: + // hand the lock back instead of holding it while wedged in Error. + await this._releaseWriter() } finally { this._isProcessing = false log.info("indexing start flow finished", { @@ -359,7 +463,10 @@ export class CodeIndexOrchestrator { this.overlay?.reconcile(this.cacheManager.getAllHashes()) await this.cacheManager.flush?.() await this.vectorStore.markIndexingComplete() - await this.vectorStore.close?.() + // The scan finished, but this session's file watcher keeps writing the + // store for every file change, so the session-held writer lock stays + // acquired until shutdown or clearIndexData (see _writer). Releasing here + // would let a second session acquire it and scan concurrently. this.fileWatcher.setCollecting(true) this.stateManager.setSystemState("Indexed", "File watcher started. Index up-to-date.") log.info("workspace scan finalized", { @@ -385,13 +492,23 @@ export class CodeIndexOrchestrator { public async shutdown(): Promise { this._cancelRequested = true this.scanner.cancel() + // Stop the session's own watcher writes before releasing the writer lock: + // the lock releases below, so the store must be quiet by then rather than + // still receiving file-watcher batches. + if (this.fileWatcher.shutdown) await this.fileWatcher.shutdown() + else this.fileWatcher.dispose() this.fileWatcher.setCollecting(false) + // Wait out the in-flight scan before closing the store: a scan that is + // still running would lazily reconnect on its next getDb()/getTable() call, + // reopening a connection close() just tore down and leaking it. Closing + // after _active settles keeps the guarantee without racing the scan. The + // next session cannot interleave here: it can only acquire the writer lock + // once _releaseWriter runs below. await this._active + await this.vectorStore.close?.() + await this._releaseWriter() for (const sub of this._fileWatcherSubscriptions) sub.dispose() this._fileWatcherSubscriptions = [] - if (this.fileWatcher.shutdown) await this.fileWatcher.shutdown() - else this.fileWatcher.dispose() - await this.vectorStore.close?.() this._isProcessing = false } @@ -413,17 +530,67 @@ export class CodeIndexOrchestrator { log.info("cancelling indexing", { workspacePath: this.workspacePath }) this._cancelRequested = true this.scanner.cancel() - this.stopWatcher() + // Keep the file watcher alive: cancel only aborts the scan, and the + // session still holds the writer lock, so watcher batches continue to own + // the store. Disposing here (via stopWatcher) would strand _active's + // finally with a disposed watcher while file changes keep arriving. this.stateManager.setSystemState("Standby", "Indexing cancelled.") this._isProcessing = false log.info("indexing cancelled", { workspacePath: this.workspacePath }) } public async clearIndexData(): Promise { + // Reuse the session-held lock when this session owns it: acquiring again + // would contend with ourselves. Whatever the source, the watcher is + // stopped below, so no writes remain and the lock is released afterwards. + const writer = await this._acquire() + if (!writer) { + // A user-requested clear must not vanish silently when another session + // holds the lock: surface contention as an error event with a + // clear-specific location. The manager only auto-retries startIndexing + // locations, so this stays terminal: the user sees the failure and can + // retry the clear once the other session finishes. + this.emitError( + "orchestrator:clearIndexData", + new Error("Another session is indexing this workspace; index data was not cleared."), + "scan", + ) + this.stateManager.setSystemState("Error", "Another session is indexing this workspace; index data was not cleared.") + return + } + this._isProcessing = true log.info("clearing index data", { workspacePath: this.workspacePath }) try { + // A clear must not race an in-flight scan. Cancel it and wait for the + // scan to settle before deleting the collection: otherwise batches still + // running can re-add points after the clear and write under a lock this + // method is about to release (shutdown() waits the same way). + this._cancelRequested = true + this.scanner.cancel() + await this._active + // Verify ownership before deleting even when a writer is still cached: + // a heartbeat lapse during the awaited scan can let another session + // reclaim the lock while _writer stays set. _acquire() re-checks owned() + // and reuses the cached writer only when this session still holds it. + const retry = await this._acquire() + if (!retry) { + this.emitError( + "orchestrator:clearIndexData", + new Error("Another session is indexing this workspace; index data was not cleared."), + "scan", + ) + this.stateManager.setSystemState( + "Error", + "Another session is indexing this workspace; index data was not cleared.", + ) + return + } + // The watcher's drainTask is separate from _active: shutdown() waits for + // it before returning, so a batch already writing cannot upsert after + // deleteCollection() or after the lock is released below. + if (this.fileWatcher.shutdown) await this.fileWatcher.shutdown() this.stopWatcher() try { @@ -444,6 +611,9 @@ export class CodeIndexOrchestrator { } } finally { this._isProcessing = false + // stopWatcher() above means this session no longer writes the store, so + // a session-held lock must be handed back rather than kept until exit. + await this._releaseWriter() log.info("finished clearing index data", { workspacePath: this.workspacePath, state: this.stateManager.state, diff --git a/packages/kilo-indexing/src/indexing/processors/file-watcher.ts b/packages/kilo-indexing/src/indexing/processors/file-watcher.ts index 839940a9a408..87bb87f9b531 100644 --- a/packages/kilo-indexing/src/indexing/processors/file-watcher.ts +++ b/packages/kilo-indexing/src/indexing/processors/file-watcher.ts @@ -317,8 +317,10 @@ export class FileWatcher implements IFileWatcher { */ async shutdown(): Promise { this.collecting = false - if (this.batchProcessDebounceTimer) clearTimeout(this.batchProcessDebounceTimer) - this.batchProcessDebounceTimer = undefined + if (this.batchProcessDebounceTimer) { + clearTimeout(this.batchProcessDebounceTimer) + this.batchProcessDebounceTimer = undefined + } const subscription = this.subscription this.subscription = undefined await subscription diff --git a/packages/kilo-indexing/src/indexing/vector-store/lancedb-vector-store.ts b/packages/kilo-indexing/src/indexing/vector-store/lancedb-vector-store.ts index 5d131bacdc36..bda2591893e8 100644 --- a/packages/kilo-indexing/src/indexing/vector-store/lancedb-vector-store.ts +++ b/packages/kilo-indexing/src/indexing/vector-store/lancedb-vector-store.ts @@ -2,7 +2,7 @@ import { createHash } from "crypto" import * as path from "path" import type { Connection, Table, VectorQuery } from "@lancedb/lancedb" import type { IVectorStore } from "../interfaces/vector-store" -import type { Payload, VectorStoreSearchResult } from "../interfaces" +import type { Payload, StoreState, VectorStoreSearchResult } from "../interfaces" import { DEFAULT_MAX_SEARCH_RESULTS, DEFAULT_SEARCH_MIN_SCORE, @@ -651,34 +651,40 @@ export class LanceDBVectorStore implements IVectorStore { } /** - * Checks if the collection exists and has indexed points - * @returns Promise resolving to boolean indicating if the collection exists and has points + * Reports how far along the store is: empty, partially built, or + * complete. */ - async hasIndexedData(): Promise { - try { - const db = await this.getDb() - const table = await this.getTable() - const pointCount = await table.countRows() - if (pointCount === 0) { - log.info("LanceDB has no indexed data", { - workspacePath: this.workspacePath, - reason: "points_zero", - }) - return false - } - const metadataTable = await native(this.dbPath, () => db.openTable(this.metadataTableName)) - const metadataResults = await metadataTable.query().where(`key = '${KEY.complete}'`).toArray() - const indexed = metadataResults.length > 0 ? String(metadataResults[0].value) === "true" : false - log.info("LanceDB indexing metadata evaluated", { + async state(): Promise { + const db = await this.getDb() + const tables = await db.tableNames() + if (!tables.includes(this.vectorTableName)) return { status: "empty" } + + const table = await native(this.dbPath, () => db.openTable(this.vectorTableName)) + const points = await table.countRows() + if (points === 0) { + log.info("LanceDB has no indexed data", { workspacePath: this.workspacePath, - pointCount, - indexed, + reason: "points_zero", }) - return indexed - } catch (error) { - log.error("Failed to check if collection has data", { error }) - throw error + return { status: "empty", points } } + + if (!tables.includes(this.metadataTableName)) { + log.info("LanceDB index has points but no metadata table", { + workspacePath: this.workspacePath, + points, + }) + return { status: "partial", points } + } + + const rows = (await this._getMetadataValue(db, KEY.complete)) ?? "false" + const status = String(rows) === "true" ? "complete" : "partial" + log.info("LanceDB indexing metadata evaluated", { + workspacePath: this.workspacePath, + points, + status, + }) + return { status, points } } private async _upsertMetadata(metadataTable: Table, key: string, value: unknown): Promise { diff --git a/packages/kilo-indexing/src/indexing/vector-store/qdrant-client.ts b/packages/kilo-indexing/src/indexing/vector-store/qdrant-client.ts index ca3ee84641ef..99967fe5bb55 100644 --- a/packages/kilo-indexing/src/indexing/vector-store/qdrant-client.ts +++ b/packages/kilo-indexing/src/indexing/vector-store/qdrant-client.ts @@ -2,7 +2,7 @@ import { QdrantClient, type Schemas } from "@qdrant/js-client-rest" import { createHash } from "crypto" import * as path from "path" import type { IVectorStore } from "../interfaces/vector-store" -import type { Payload, VectorStoreSearchResult } from "../interfaces" +import type { Payload, StoreState, VectorStoreSearchResult } from "../interfaces" import { DEFAULT_MAX_SEARCH_RESULTS, DEFAULT_SEARCH_MIN_SCORE } from "../constants" import { Log } from "../../util/log" import type { EmbeddingProfile } from "../embedding-profile" @@ -674,44 +674,39 @@ export class QdrantVectorStore implements IVectorStore { } /** - * Checks if the collection exists and has indexed points - * @returns Promise resolving to boolean indicating if the collection exists and has points + * Reports how far along the store is: empty, partially built, or + * complete. */ - async hasIndexedData(): Promise { - try { - const collectionInfo = await this.client.getCollection(this.collectionName) - // Check if the collection has any points indexed - const pointsCount = collectionInfo.points_count ?? 0 - if (pointsCount === 0) { - log.info("Qdrant collection has no indexed data", { - collection: this.collectionName, - reason: "points_zero", - }) - return false - } - - // Check if the indexing completion marker exists - const payload = await this.getMetadataPayload() - - // If marker exists, use it to determine completion status - if (payload) { - const indexed = payload[KEY.complete] === true - log.info("Qdrant indexing metadata evaluated", { - collection: this.collectionName, - pointsCount, - indexed, - }) - return indexed - } + async state(): Promise { + const collectionInfo = await this.client.getCollection(this.collectionName) + const points = collectionInfo.points_count ?? 0 + // The completion marker is itself a point, so subtract it before judging + // emptiness: otherwise a scanned-then-cleared collection reports + // points_count 1 and `empty` is unreachable after any scan. + const payload = await this.getMetadataPayload() + const indexed = Math.max(0, points - (payload ? 1 : 0)) + if (indexed === 0) { + log.info("Qdrant collection has no indexed data", { + collection: this.collectionName, + reason: "points_zero", + }) + return { status: "empty", points: indexed } + } - // Backward compatibility: No marker exists (old index or pre-marker version) - // Fall back to old logic - assume complete if collection has points - log.info("No indexing metadata marker found. Using backward compatibility mode (checking points_count > 0).") - return pointsCount > 0 - } catch (error) { - log.error("Failed to check if collection has data", { error }) - throw error + if (payload) { + const status = payload[KEY.complete] === true ? "complete" : "partial" + log.info("Qdrant indexing metadata evaluated", { + collection: this.collectionName, + points: indexed, + status, + }) + return { status, points: indexed } } + + // Backward compatibility: no marker means an index from before the marker + // existed, which can only ever have been written by a finished scan. + log.info("No indexing metadata marker found. Using backward compatibility mode (checking points_count > 0).") + return { status: "complete", points: indexed } } /** diff --git a/packages/kilo-indexing/src/indexing/writer-lock.ts b/packages/kilo-indexing/src/indexing/writer-lock.ts new file mode 100644 index 000000000000..329dc8077cef --- /dev/null +++ b/packages/kilo-indexing/src/indexing/writer-lock.ts @@ -0,0 +1,214 @@ +import { createHash, randomUUID } from "crypto" +import fs from "fs/promises" +import os from "os" +import path from "path" +import { Log } from "../util/log" + +const log = Log.create({ service: "indexing-writer-lock" }) + +const STALE_MS = 60_000 +const BEAT_MS = Math.floor(STALE_MS / 3) + +export type Writer = { + release: () => Promise + owned: () => Promise +} + +type Meta = { + token: string + pid: number + host: string +} + +function key(workspacePath: string): string { + return createHash("sha256").update(workspacePath).digest("hex").substring(0, 16) +} + +function code(err: unknown): string | undefined { + if (typeof err !== "object" || err === null || !("code" in err)) return undefined + return typeof err.code === "string" ? err.code : undefined +} + +function alive(pid: number): boolean { + try { + process.kill(pid, 0) + return true + } catch (err) { + return code(err) === "EPERM" + } +} + +async function age(file: string): Promise { + try { + return Date.now() - (await fs.stat(file)).mtimeMs + } catch (err) { + if (code(err) === "ENOENT") return undefined + throw err + } +} + +async function read(file: string): Promise { + try { + const parsed: unknown = JSON.parse(await fs.readFile(file, "utf-8")) + if (typeof parsed !== "object" || parsed === null) return undefined + if (!("token" in parsed) || !("pid" in parsed) || !("host" in parsed)) return undefined + return { token: String(parsed.token), pid: Number(parsed.pid), host: String(parsed.host) } + } catch (err) { + log.error("writer lock metadata unreadable", { err, file }) + return undefined + } +} + +async function claim(lockdir: string): Promise { + try { + await fs.mkdir(lockdir) + return true + } catch (err) { + if (code(err) === "EEXIST") return false + throw err + } +} + +/** + * A lock whose owner died leaves its directory behind forever, so ownership is + * decided by liveness rather than presence: a heartbeat that stopped, or a + * local pid that no longer exists, means the directory is safe to take. + * + * A vanished directory reports as reclaimable: without this, a session that + * lost the race on `claim` would treat its own missing directory (deleted by + * the winner's reclaim just before its claim) as held-forever and wedge in + * Standby instead of retrying the claim. + */ +async function reclaimable(lockdir: string): Promise { + const beat = path.join(lockdir, "heartbeat") + const meta = path.join(lockdir, "meta.json") + + // Check the dead-local-owner fast path before heartbeat freshness: a dead + // pid cannot refresh its heartbeat, so waiting out STALE_MS on a fresh file + // only delays an inevitable reclaim. (Pid reuse is safe: a reused pid reads + // alive and falls through to the age checks.) + const owner = await read(meta) + if (owner && owner.host === os.hostname() && !alive(owner.pid)) { + log.info("reclaiming writer lock from a dead local owner", { pid: owner.pid }) + return true + } + + const beatAge = await age(beat) + if (beatAge !== undefined) return beatAge > STALE_MS + + const metaAge = await age(meta) + if (metaAge !== undefined) return metaAge > STALE_MS + + const dirAge = await age(lockdir) + if (dirAge === undefined) return true + return dirAge > STALE_MS +} + +/** + * Takes the writer lock for a workspace, or resolves undefined when another + * live session already holds it. Callers must not write to the vector store + * without holding this. + */ +export async function acquire(dir: string, workspacePath: string): Promise { + const root = path.join(dir, "locks") + const lockdir = path.join(root, key(workspacePath)) + + await fs.mkdir(root, { recursive: true }) + if (!(await claim(lockdir))) { + if (!(await reclaimable(lockdir))) { + log.info("writer lock held by another session", { workspacePath }) + return undefined + } + log.info("reclaiming stale writer lock", { workspacePath }) + // Atomic reclaim: rename (same-filesystem atomic) instead of rm+mkdir, so + // two reclaimers cannot both delete each other's fresh claim and both + // become writers. The loser of the rename claims from scratch: if its claim + // loses, the directory it sees is the winner's fresh lock, which reports + // held rather than free. + if (!(await reclaim(lockdir))) { + log.info("lost the race for a stale writer lock", { workspacePath }) + return undefined + } + } + + const token = randomUUID() + const owner = { token, pid: process.pid, host: os.hostname(), at: new Date().toISOString() } + try { + await fs.writeFile(path.join(lockdir, "meta.json"), JSON.stringify(owner)) + await fs.writeFile(path.join(lockdir, "heartbeat"), "") + } catch (err) { + // A failed write after winning the claim must not leave the lock held and + // fresh: roll the directory back so a later session sees it as stale or + // retries the claim, and surface the error instead of an unhandled + // rejection escaping runIndexing. + await fs.rm(lockdir, { recursive: true, force: true }) + throw err + } + + let held = true + const timer = setInterval(() => { + if (!held) return + const now = new Date() + void (async () => { + // Stop beating once displaced: without the ownership check a taken-over + // owner keeps refreshing the new owner's heartbeat and may resume + // writing while it believes the lock is still its own. + const current = await read(path.join(lockdir, "meta.json")) + if (!current || current.token !== token) { + held = false + clearInterval(timer) + return + } + await fs.utimes(path.join(lockdir, "heartbeat"), now, now) + })().catch((err) => log.error("writer lock heartbeat failed", { err })) + }, BEAT_MS) + timer.unref?.() + + // A caller may cache this writer across runs. Reading the metadata rather + // than trusting the heartbeat timer lets it learn about a takeover before + // writing the store under a lock another session now owns. + const owned = async () => { + // A single unreadable metadata read (transient IO, a writer mid-rewrite) + // must not permanently park the session and dispose its watcher. Retry + // once; a real takeover leaves a stable foreign token on the second read. + for (let attempt = 0; attempt < 2; attempt += 1) { + const current = await read(path.join(lockdir, "meta.json")) + if (current) return current.token === token + if (attempt === 0) await new Promise((resolve) => setTimeout(resolve, 25)) + } + return false + } + + return { + owned, + release: async () => { + held = false + clearInterval(timer) + const current = await read(path.join(lockdir, "meta.json")) + if (!current || current.token !== token) { + // The directory (or its meta) belongs to another session now: the + // takeover rewrote it, or it vanished and was reclaimed. Either way + // deleting it would remove a lock this session no longer owns. + if (current) log.error("refusing to release a writer lock owned by another session", { workspacePath }) + return + } + await fs.rm(lockdir, { recursive: true, force: true }) + log.info("released writer lock", { workspacePath }) + }, + } +} + +async function reclaim(lockdir: string): Promise { + const stale = `${lockdir}.stale.${process.pid}.${Date.now()}` + try { + await fs.rename(lockdir, stale) + } catch (err) { + // ENOENT means the directory vanished between the reclaimable check and + // the rename — i.e. the lock is free, not lost to a rival. Claim it + // instead of reporting a lost race and wedging the session in Standby. + if (code(err) !== "ENOENT") return false + return claim(lockdir) + } + void fs.rm(stale, { recursive: true, force: true }).catch((err) => log.error("writer lock stale cleanup failed", { err })) + return claim(lockdir) +} diff --git a/packages/kilo-indexing/test/kilocode/indexing/manager.test.ts b/packages/kilo-indexing/test/kilocode/indexing/manager.test.ts index 649d4a1444ea..9c64a83c686b 100644 --- a/packages/kilo-indexing/test/kilocode/indexing/manager.test.ts +++ b/packages/kilo-indexing/test/kilocode/indexing/manager.test.ts @@ -199,6 +199,11 @@ describe("CodeIndexManager", () => { } finally { await mgr.dispose() } + // The manager can leave a native watcher handle alive on Windows even + // after dispose. This is a one-shot probe, so exit explicitly instead + // of waiting for the event loop to drain; otherwise the parent sees a + // live child and times out while all assertions have already passed. + process.exit(0) ` const child = Bun.spawn([process.execPath, "-e", script], { cwd: join(import.meta.dir, "../../.."), @@ -210,7 +215,7 @@ describe("CodeIndexManager", () => { const timeout = setTimeout(() => { child.kill() gate.reject(new Error("Indexing subprocess timed out")) - }, 10_000) + }, 30_000) const [exit, stderr] = await Promise.race([ Promise.all([child.exited, new Response(child.stderr).text()]), gate.promise, diff --git a/packages/kilo-indexing/test/kilocode/indexing/orchestrator.test.ts b/packages/kilo-indexing/test/kilocode/indexing/orchestrator.test.ts index f2fc3f046de0..a75f2ae476dd 100644 --- a/packages/kilo-indexing/test/kilocode/indexing/orchestrator.test.ts +++ b/packages/kilo-indexing/test/kilocode/indexing/orchestrator.test.ts @@ -1,4 +1,8 @@ import { describe, expect, test } from "bun:test" +import { mkdir, mkdtemp, rm, writeFile } from "fs/promises" +import { createHash } from "crypto" +import { hostname, tmpdir } from "os" +import path from "path" import { CodeIndexConfigManager } from "../../../src/indexing/config-manager" import { CodeIndexOrchestrator } from "../../../src/indexing/orchestrator" import { CodeIndexStateManager } from "../../../src/indexing/state-manager" @@ -11,9 +15,12 @@ import type { IndexingTelemetryEvent, IVectorStore, PointStruct, + StoreState, + StoreStatus, VectorStoreSearchResult, } from "../../../src/indexing/interfaces" import { Emitter } from "../../../src/indexing/runtime" +import { acquire } from "../../../src/indexing/writer-lock" class Store { public clearCount = 0 @@ -23,7 +30,7 @@ class Store { public incompleteCount = 0 constructor( - private readonly existing: boolean, + private readonly status: StoreStatus, private readonly created = false, ) {} @@ -56,8 +63,8 @@ class Store { async collectionExists(): Promise { return true } - async hasIndexedData(): Promise { - return this.existing + async state(): Promise { + return { status: this.status } } async markIndexingComplete(): Promise { this.completeCount += 1 @@ -100,6 +107,9 @@ class Scanner { } class Watcher { + public disposed = 0 + public shutdowns = 0 + public onShutdown?: () => Promise public readonly onDidStartBatchProcessing = new Emitter() public readonly onBatchProgressUpdate = new Emitter<{ processedInBatch: number @@ -120,7 +130,13 @@ class Watcher { } } + async shutdown(): Promise { + this.shutdowns += 1 + await this.onShutdown?.() + } + dispose(): void { + this.disposed += 1 this.onDidStartBatchProcessing.dispose() this.onBatchProgressUpdate.dispose() this.onDidFinishBatchProcessing.dispose() @@ -148,6 +164,47 @@ class BlockingScanner { updateBatchSegmentThreshold(_newThreshold: number): void {} } +class StubbornScanner { + public isCancelled = false + public finished = false + readonly started = Promise.withResolvers() + readonly gate = Promise.withResolvers() + + async scanDirectory(): Promise<{ stats: { processed: number; skipped: number }; totalBlockCount: number }> { + this.started.resolve() + await this.gate.promise + this.finished = true + return { stats: { processed: 0, skipped: 0 }, totalBlockCount: 0 } + } + + cancel(): void { + // A real batch already in flight finishes its current work even after + // cancellation; only the caller releasing the gate ends this scan. + this.isCancelled = true + } + + updateBatchSegmentThreshold(_newThreshold: number): void {} +} + +class FailingBlockingScanner { + public isCancelled = false + readonly started = Promise.withResolvers() + readonly gate = Promise.withResolvers() + + async scanDirectory(): Promise<{ stats: { processed: number; skipped: number }; totalBlockCount: number }> { + this.started.resolve() + await this.gate.promise + throw new Error("scan failed") + } + + cancel(): void { + this.isCancelled = true + this.gate.resolve() + } + + updateBatchSegmentThreshold(_newThreshold: number): void {} +} + class FailScanner { public readonly isCancelled = false @@ -179,7 +236,7 @@ describe("CodeIndexOrchestrator telemetry", () => { { async clearCacheFile() {}, } as unknown as CacheManager, - new Store(false) as unknown as IVectorStore, + new Store("empty") as unknown as IVectorStore, new Scanner(3, 3, 6) as unknown as DirectoryScanner, new Watcher() as unknown as IFileWatcher, (event) => events.push(event), @@ -207,7 +264,7 @@ describe("CodeIndexOrchestrator telemetry", () => { { async clearCacheFile() {}, } as unknown as CacheManager, - new Store(true) as unknown as IVectorStore, + new Store("complete") as unknown as IVectorStore, new Scanner(2, 1, 2) as unknown as DirectoryScanner, new Watcher() as unknown as IFileWatcher, (event) => events.push(event), @@ -240,7 +297,7 @@ describe("CodeIndexOrchestrator telemetry", () => { new CodeIndexStateManager(), "/tmp/ws", { async clearCacheFile() {} } as unknown as CacheManager, - new Store(false) as unknown as IVectorStore, + new Store("empty") as unknown as IVectorStore, scanner, new Watcher() as unknown as IFileWatcher, ) @@ -258,7 +315,7 @@ describe("CodeIndexOrchestrator telemetry", () => { test("shutdown waits for an active scan before closing the store", async () => { const scanner = new BlockingScanner() - const store = new Store(false) + const store = new Store("empty") const orchestrator = new CodeIndexOrchestrator( createConfig(), new CodeIndexStateManager(), @@ -282,7 +339,7 @@ describe("CodeIndexOrchestrator telemetry", () => { test("preserves an unchanged index when an incremental scan is interrupted", async () => { const scanner = new BlockingScanner() - const store = new Store(true) + const store = new Store("complete") const orchestrator = new CodeIndexOrchestrator( createConfig(), new CodeIndexStateManager(), @@ -303,7 +360,7 @@ describe("CodeIndexOrchestrator telemetry", () => { expect(store.clearCount).toBe(0) }) - test("clears stale vectors and hashes before rebuilding an incomplete store", async () => { + test("clears stale vectors and hashes before rebuilding an empty store", async () => { const cache = { clears: 0, async clearCacheFile() { @@ -311,7 +368,7 @@ describe("CodeIndexOrchestrator telemetry", () => { }, async flush() {}, } - const store = new Store(false, false) + const store = new Store("empty", false) const orchestrator = new CodeIndexOrchestrator( createConfig(), new CodeIndexStateManager(), @@ -329,6 +386,113 @@ describe("CodeIndexOrchestrator telemetry", () => { expect(orchestrator.state).toBe("Indexed") }) + test("does not clear a partially built index", async () => { + const cache = { + clears: 0, + async clearCacheFile() { + this.clears += 1 + }, + async flush() {}, + } + const store = new Store("partial", false) + const events: IndexingTelemetryEvent[] = [] + const orchestrator = new CodeIndexOrchestrator( + createConfig(), + new CodeIndexStateManager(), + "/tmp/ws", + cache as unknown as CacheManager, + store as unknown as IVectorStore, + new Scanner(2, 2, 4) as unknown as DirectoryScanner, + new Watcher() as unknown as IFileWatcher, + (event) => events.push(event), + ) + + await orchestrator.startIndexing("background") + + const completed = events.find( + (event): event is Extract => event.type === "completed", + ) + expect(store.clearCount).toBe(0) + expect(cache.clears).toBe(0) + expect(completed?.mode).toBe("full") + expect(orchestrator.state).toBe("Indexed") + }) + + test("stands by while another session holds the writer lock", async () => { + const dir = await mkdtemp(path.join(tmpdir(), "indexing-lock-")) + const held = await acquire(dir, "/tmp/ws") + expect(held).toBeDefined() + + const events: IndexingTelemetryEvent[] = [] + const store = new Store("partial", false) + let scanned = false + const scanner = new Scanner(1, 1, 1) as unknown as DirectoryScanner + const scan = scanner.scanDirectory.bind(scanner) + scanner.scanDirectory = async (...args: Parameters) => { + scanned = true + return scan(...args) + } + const orchestrator = new CodeIndexOrchestrator( + createConfig(), + new CodeIndexStateManager(), + "/tmp/ws", + { dir, async clearCacheFile() {}, async flush() {} } as unknown as CacheManager, + store as unknown as IVectorStore, + scanner, + new Watcher() as unknown as IFileWatcher, + (event) => events.push(event), + ) + + await orchestrator.startIndexing("background") + + expect(scanned).toBe(false) + expect(store.clearCount).toBe(0) + expect(store.deleteCount).toBe(0) + expect(store.incompleteCount).toBe(0) + expect(orchestrator.state).toBe("Standby") + // Contention is observable (not a silent park) and never consumes the + // manager's failure-retry budget: it reports the contention text while a + // real scan error reports its own message. + const error = events.find( + (event): event is Extract => event.type === "error", + ) + expect(error?.error).toContain("Another session is indexing this workspace") + + await held?.release() + await rm(dir, { recursive: true, force: true }) + }) + + test("contended clear reports an error instead of silent Standby", async () => { + const dir = await mkdtemp(path.join(tmpdir(), "indexing-lock-")) + const held = await acquire(dir, "/tmp/ws") + expect(held).toBeDefined() + + const events: IndexingTelemetryEvent[] = [] + const store = new Store("complete", false) + const orchestrator = new CodeIndexOrchestrator( + createConfig(), + new CodeIndexStateManager(), + "/tmp/ws", + { dir, async clearCacheFile() {}, async flush() {} } as unknown as CacheManager, + store as unknown as IVectorStore, + new Scanner(1, 1, 1) as unknown as DirectoryScanner, + new Watcher() as unknown as IFileWatcher, + (event) => events.push(event), + ) + + await orchestrator.clearIndexData() + + expect(store.deleteCount).toBe(0) + expect(orchestrator.state).toBe("Error") + const error = events.find( + (event): event is Extract => event.type === "error", + ) + expect(error?.error).toContain("was not cleared") + + await held?.release() + await rm(dir, { recursive: true, force: true }) + }) + test("does not clear data when index completeness cannot be read", async () => { const cache = { clears: 0, @@ -336,8 +500,8 @@ describe("CodeIndexOrchestrator telemetry", () => { this.clears += 1 }, } - const store = new Store(true, false) - store.hasIndexedData = async () => { + const store = new Store("complete", false) + store.state = async () => { throw new Error("metadata unavailable") } const orchestrator = new CodeIndexOrchestrator( @@ -365,7 +529,7 @@ describe("CodeIndexOrchestrator telemetry", () => { this.clears += 1 }, } - const store = new Store(true) + const store = new Store("complete") const orchestrator = new CodeIndexOrchestrator( createConfig(), new CodeIndexStateManager(), @@ -390,4 +554,269 @@ describe("CodeIndexOrchestrator telemetry", () => { expect(store.deleteCount).toBe(0) expect(orchestrator.state).toBe("Error") }) + + test("holds the writer lock across runs until shutdown", async () => { + const dir = await mkdtemp(path.join(tmpdir(), "indexing-lock-")) + const store = new Store("empty", false) + const orchestrator = new CodeIndexOrchestrator( + createConfig(), + new CodeIndexStateManager(), + "/tmp/ws", + { dir, async clearCacheFile() {}, async flush() {} } as unknown as CacheManager, + store as unknown as IVectorStore, + new Scanner(1, 1, 1) as unknown as DirectoryScanner, + new Watcher() as unknown as IFileWatcher, + ) + + await orchestrator.startIndexing("background") + expect(orchestrator.state).toBe("Indexed") + // The lock stays held after the scan for the watcher's ongoing writes: a + // rival session must still see contention instead of scanning concurrently. + expect(await acquire(dir, "/tmp/ws")).toBeUndefined() + + // A second run reuses the session-held lock instead of contending with itself. + await orchestrator.startIndexing("background") + expect(orchestrator.state).toBe("Indexed") + + await orchestrator.shutdown() + // Shutdown hands the lock back: a new session can now acquire it. + const next = await acquire(dir, "/tmp/ws") + expect(next).toBeDefined() + await next?.release() + await rm(dir, { recursive: true, force: true }) + }) + + test("releases the session-held writer lock after clear", async () => { + const dir = await mkdtemp(path.join(tmpdir(), "indexing-lock-")) + const store = new Store("empty", false) + const orchestrator = new CodeIndexOrchestrator( + createConfig(), + new CodeIndexStateManager(), + "/tmp/ws", + { dir, async clearCacheFile() {}, async flush() {} } as unknown as CacheManager, + store as unknown as IVectorStore, + new Scanner(1, 1, 1) as unknown as DirectoryScanner, + new Watcher() as unknown as IFileWatcher, + ) + + await orchestrator.startIndexing("background") + expect(orchestrator.state).toBe("Indexed") + + await orchestrator.clearIndexData() + // The watcher is stopped by clear, so no writes remain: keeping the lock + // would starve every other session for the rest of this one's lifetime. + const next = await acquire(dir, "/tmp/ws") + expect(next).toBeDefined() + await next?.release() + await rm(dir, { recursive: true, force: true }) + }) + + test("waits for the active scan before clearing data", async () => { + const dir = await mkdtemp(path.join(tmpdir(), "indexing-lock-")) + const scanner = new StubbornScanner() + const store = new Store("complete", false) + const orchestrator = new CodeIndexOrchestrator( + createConfig(), + new CodeIndexStateManager(), + "/tmp/ws", + { dir, async clearCacheFile() {}, async flush() {} } as unknown as CacheManager, + store as unknown as IVectorStore, + scanner as unknown as DirectoryScanner, + new Watcher() as unknown as IFileWatcher, + ) + + const active = orchestrator.startIndexing("background") + await scanner.started.promise + const clearing = orchestrator.clearIndexData() + for (let i = 0; i < 200 && !scanner.isCancelled; i += 1) await Bun.sleep(5) + // The in-flight batch is still running, so nothing may be deleted yet. + expect(scanner.isCancelled).toBe(true) + expect(store.deleteCount).toBe(0) + + scanner.gate.resolve() + await clearing + await active + expect(store.deleteCount).toBe(1) + await rm(dir, { recursive: true, force: true }) + }) + + test("re-acquires the writer lock before clearing after a failed scan", async () => { + const dir = await mkdtemp(path.join(tmpdir(), "indexing-lock-")) + const scanner = new FailingBlockingScanner() + const store = new Store("complete", false) + let heldDuringDelete = false + const remove = store.deleteCollection.bind(store) + store.deleteCollection = async () => { + heldDuringDelete = (await acquire(dir, "/tmp/ws")) === undefined + await remove() + } + const orchestrator = new CodeIndexOrchestrator( + createConfig(), + new CodeIndexStateManager(), + "/tmp/ws", + { dir, async clearCacheFile() {}, async flush() {} } as unknown as CacheManager, + store as unknown as IVectorStore, + scanner as unknown as DirectoryScanner, + new Watcher() as unknown as IFileWatcher, + ) + + const active = orchestrator.startIndexing("background") + await scanner.started.promise + // The scan's error path releases the session lock; the clear must take it + // again rather than deleting the collection unlocked. + await orchestrator.clearIndexData() + await active + + expect(store.deleteCount).toBe(1) + expect(heldDuringDelete).toBe(true) + const next = await acquire(dir, "/tmp/ws") + expect(next).toBeDefined() + await next?.release() + await rm(dir, { recursive: true, force: true }) + }) + + test("waits for the watcher drain before clearing data", async () => { + const dir = await mkdtemp(path.join(tmpdir(), "indexing-lock-")) + const store = new Store("complete", false) + const watcher = new Watcher() + const drain = Promise.withResolvers() + watcher.onShutdown = () => drain.promise + const orchestrator = new CodeIndexOrchestrator( + createConfig(), + new CodeIndexStateManager(), + "/tmp/ws", + { dir, async clearCacheFile() {}, async flush() {} } as unknown as CacheManager, + store as unknown as IVectorStore, + new Scanner(1, 1, 1) as unknown as DirectoryScanner, + watcher as unknown as IFileWatcher, + ) + + await orchestrator.startIndexing("background") + const clearing = orchestrator.clearIndexData() + for (let i = 0; i < 200 && watcher.shutdowns === 0; i += 1) await Bun.sleep(5) + // The watcher's drain task is still writing, so the collection stays. + expect(watcher.shutdowns).toBe(1) + expect(store.deleteCount).toBe(0) + + drain.resolve() + await clearing + expect(store.deleteCount).toBe(1) + await rm(dir, { recursive: true, force: true }) + }) + + test("stops the watcher when the writer lock is lost during a batch", async () => { + const dir = await mkdtemp(path.join(tmpdir(), "indexing-lock-")) + const store = new Store("complete", false) + const watcher = new Watcher() + const orchestrator = new CodeIndexOrchestrator( + createConfig(), + new CodeIndexStateManager(), + "/tmp/ws", + { dir, async clearCacheFile() {}, async flush() {} } as unknown as CacheManager, + store as unknown as IVectorStore, + new Scanner(1, 1, 1) as unknown as DirectoryScanner, + watcher as unknown as IFileWatcher, + ) + + await orchestrator.startIndexing("background") + expect(orchestrator.state).toBe("Indexed") + + const slot = path.join(dir, "locks", createHash("sha256").update("/tmp/ws").digest("hex").substring(0, 16)) + await rm(slot, { recursive: true, force: true }) + await mkdir(slot, { recursive: true }) + await writeFile(path.join(slot, "meta.json"), JSON.stringify({ token: "rival", pid: process.pid, host: hostname() })) + await writeFile(path.join(slot, "heartbeat"), "") + + watcher.onDidStartBatchProcessing.fire(["src/a.ts"]) + for (let i = 0; i < 100 && watcher.disposed === 0; i += 1) await Bun.sleep(10) + // The watcher writes the store outside runIndexing, so a batch after the + // takeover must shut it down rather than commit unlocked. + expect(watcher.disposed).toBeGreaterThan(0) + expect(orchestrator.state).toBe("Error") + await rm(dir, { recursive: true, force: true }) + }) + + test("stops writing when another session takes over the cached lock", async () => { + const dir = await mkdtemp(path.join(tmpdir(), "indexing-lock-")) + const store = new Store("complete", false) + const orchestrator = new CodeIndexOrchestrator( + createConfig(), + new CodeIndexStateManager(), + "/tmp/ws", + { dir, async clearCacheFile() {}, async flush() {} } as unknown as CacheManager, + store as unknown as IVectorStore, + new Scanner(1, 1, 1) as unknown as DirectoryScanner, + new Watcher() as unknown as IFileWatcher, + ) + + await orchestrator.startIndexing("background") + expect(orchestrator.state).toBe("Indexed") + + const slot = path.join(dir, "locks", createHash("sha256").update("/tmp/ws").digest("hex").substring(0, 16)) + const before = store.deleteCount + await rm(slot, { recursive: true, force: true }) + await mkdir(slot, { recursive: true }) + await writeFile(path.join(slot, "meta.json"), JSON.stringify({ token: "rival", pid: process.pid, host: hostname() })) + await writeFile(path.join(slot, "heartbeat"), "") + + await orchestrator.clearIndexData() + // The cached writer is no longer the owner, so the clear must not write + // the store. It re-acquires and reports contention instead. + expect(store.deleteCount).toBe(before) + expect(orchestrator.state).toBe("Error") + await rm(dir, { recursive: true, force: true }) + }) + + test("releases the writer lock when a run fails", async () => { + const dir = await mkdtemp(path.join(tmpdir(), "indexing-lock-")) + const store = new Store("empty", false) + const orchestrator = new CodeIndexOrchestrator( + createConfig(), + new CodeIndexStateManager(), + "/tmp/ws", + { dir, async clearCacheFile() {}, async flush() {} } as unknown as CacheManager, + store as unknown as IVectorStore, + new FailScanner() as unknown as DirectoryScanner, + new Watcher() as unknown as IFileWatcher, + ) + + await orchestrator.startIndexing("background") + expect(orchestrator.state).toBe("Error") + // The watcher is stopped on error, so the lock must not stay held while wedged. + const next = await acquire(dir, "/tmp/ws") + expect(next).toBeDefined() + await next?.release() + await rm(dir, { recursive: true, force: true }) + }) + + test("contended clear reports a clear location that the manager does not retry", async () => { + const dir = await mkdtemp(path.join(tmpdir(), "indexing-lock-")) + const held = await acquire(dir, "/tmp/ws") + expect(held).toBeDefined() + + const events: IndexingTelemetryEvent[] = [] + const store = new Store("complete", false) + const orchestrator = new CodeIndexOrchestrator( + createConfig(), + new CodeIndexStateManager(), + "/tmp/ws", + { dir, async clearCacheFile() {}, async flush() {} } as unknown as CacheManager, + store as unknown as IVectorStore, + new Scanner(1, 1, 1) as unknown as DirectoryScanner, + new Watcher() as unknown as IFileWatcher, + (event) => events.push(event), + ) + + await orchestrator.clearIndexData() + + const error = events.find( + (event): event is Extract => event.type === "error", + ) + // A clear-specific location keeps the manager's startIndexing auto-recovery + // from firing: recovery would rerun a scan, not the requested clear. + expect(error?.location).toBe("orchestrator:clearIndexData") + + await held?.release() + await rm(dir, { recursive: true, force: true }) + }) }) diff --git a/packages/kilo-indexing/test/kilocode/indexing/processors/file-watcher.test.ts b/packages/kilo-indexing/test/kilocode/indexing/processors/file-watcher.test.ts index 2402a00d6855..8ab192fa47c1 100644 --- a/packages/kilo-indexing/test/kilocode/indexing/processors/file-watcher.test.ts +++ b/packages/kilo-indexing/test/kilocode/indexing/processors/file-watcher.test.ts @@ -11,6 +11,7 @@ import type { IndexingTelemetryEvent, IVectorStore, PointStruct, + StoreState, VectorStoreSearchResult, } from "../../../../src/indexing/interfaces" import { @@ -76,8 +77,8 @@ class RetryStore implements IVectorStore { async collectionExists(): Promise { return true } - async hasIndexedData(): Promise { - return false + async state(): Promise { + return { status: "empty" } } async markIndexingComplete(): Promise {} async markIndexingIncomplete(): Promise {} diff --git a/packages/kilo-indexing/test/kilocode/indexing/processors/scanner.test.ts b/packages/kilo-indexing/test/kilocode/indexing/processors/scanner.test.ts index a7d9dd04931d..cc37cf473d16 100644 --- a/packages/kilo-indexing/test/kilocode/indexing/processors/scanner.test.ts +++ b/packages/kilo-indexing/test/kilocode/indexing/processors/scanner.test.ts @@ -12,6 +12,7 @@ import type { IndexingTelemetryEvent, IVectorStore, PointStruct, + StoreState, VectorStoreSearchResult, } from "../../../../src/indexing/interfaces" import { loadIgnore } from "../../../../src/indexing/shared/load-ignore" @@ -138,8 +139,8 @@ class Store implements IVectorStore { return true } - public async hasIndexedData(): Promise { - return false + public async state(): Promise { + return { status: "empty" } } public async markIndexingComplete(): Promise {} diff --git a/packages/kilo-indexing/test/kilocode/indexing/vector-store/qdrant-client.test.ts b/packages/kilo-indexing/test/kilocode/indexing/vector-store/qdrant-client.test.ts index 6661297cb429..c5747c9bbee5 100644 --- a/packages/kilo-indexing/test/kilocode/indexing/vector-store/qdrant-client.test.ts +++ b/packages/kilo-indexing/test/kilocode/indexing/vector-store/qdrant-client.test.ts @@ -831,6 +831,26 @@ describe("QdrantVectorStore", () => { expect(mockCreatePayloadIndex).toHaveBeenCalledTimes(6) }) + test("state reports empty when only the completion marker remains", async () => { + // A scanned-then-cleared collection still holds points_count 1 for the + // completion marker: emptiness must subtract it. + mockGetCollection.mockResolvedValue({ points_count: 1 }) + mockRetrieve.mockResolvedValue([{ payload: { indexing_complete: true } }]) + + const state = await vectorStore.state() + + expect(state).toEqual({ status: "empty", points: 0 }) + }) + + test("state counts data points without the completion marker", async () => { + mockGetCollection.mockResolvedValue({ points_count: 4 }) + mockRetrieve.mockResolvedValue([{ payload: { indexing_complete: true } }]) + + const state = await vectorStore.state() + + expect(state).toEqual({ status: "complete", points: 3 }) + }) + test("should throw vectorDimensionMismatch error when deleteCollection fails during recreation", async () => { const differentVectorSize = 768 mockGetCollection.mockResolvedValue({ diff --git a/packages/kilo-indexing/test/kilocode/indexing/writer-lock.test.ts b/packages/kilo-indexing/test/kilocode/indexing/writer-lock.test.ts new file mode 100644 index 000000000000..a28e51fe6be8 --- /dev/null +++ b/packages/kilo-indexing/test/kilocode/indexing/writer-lock.test.ts @@ -0,0 +1,134 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test" +import { mkdir, mkdtemp, readFile, readdir, rm, utimes, writeFile } from "fs/promises" +import { hostname, tmpdir } from "os" +import path from "path" +import { createHash } from "crypto" +import { acquire } from "../../../src/indexing/writer-lock" + +const WS = "/tmp/workspace-under-test" + +let dir: string + +function slot(): string { + return path.join(dir, "locks", createHash("sha256").update(WS).digest("hex").substring(0, 16)) +} + +async function meta(): Promise<{ token: string; pid: number; host: string; at: string }> { + return JSON.parse(await readFile(path.join(slot(), "meta.json"), "utf-8")) +} + +async function plant(owner: { token: string; pid: number; host: string }, beat = false): Promise { + await mkdir(slot(), { recursive: true }) + await writeFile(path.join(slot(), "meta.json"), JSON.stringify({ ...owner, at: new Date().toISOString() })) + if (!beat) return + await writeFile(path.join(slot(), "heartbeat"), "") + const when = new Date(Date.now() - 120_000) + await utimes(path.join(slot(), "heartbeat"), when, when) +} + +beforeEach(async () => { + dir = await mkdtemp(path.join(tmpdir(), "writer-lock-")) +}) + +afterEach(async () => { + await rm(dir, { recursive: true, force: true }) +}) + +describe("writer lock", () => { + test("grants the lock once and refuses a second holder", async () => { + const first = await acquire(dir, WS) + expect(first).toBeDefined() + expect(await acquire(dir, WS)).toBeUndefined() + await first?.release() + }) + + test("grants the lock again after release", async () => { + const first = await acquire(dir, WS) + await first?.release() + expect(await acquire(dir, WS)).toBeDefined() + }) + + test("scopes the lock to the workspace", async () => { + const first = await acquire(dir, WS) + expect(await acquire(dir, "/tmp/some-other-workspace")).toBeDefined() + await first?.release() + }) + + test("records the owning process", async () => { + const writer = await acquire(dir, WS) + expect((await meta()).pid).toBe(process.pid) + await writer?.release() + }) + + test("reports ownership only while the lock still belongs to this writer", async () => { + const writer = await acquire(dir, WS) + expect(await writer?.owned()).toBe(true) + + // A takeover rewrites the metadata; the cached writer must notice instead + // of continuing to write the store as if the lock were still its own. + const current = await meta() + await writeFile(path.join(slot(), "meta.json"), JSON.stringify({ ...current, token: "rival" })) + expect(await writer?.owned()).toBe(false) + await writer?.release() + }) + + test("takes over a lock whose heartbeat stopped", async () => { + await plant({ token: "dead", pid: process.pid, host: "elsewhere" }, true) + + const writer = await acquire(dir, WS) + expect(writer).toBeDefined() + expect((await meta()).token).not.toBe("dead") + await writer?.release() + }) + + test("takes over a lock whose local owner is gone", async () => { + await plant({ token: "dead", pid: 2 ** 30, host: hostname() }) + + const writer = await acquire(dir, WS) + expect(writer).toBeDefined() + expect((await meta()).token).not.toBe("dead") + await writer?.release() + }) + + test("leaves a fresh lock held by a live owner alone", async () => { + await plant({ token: "live", pid: process.pid, host: hostname() }) + expect(await acquire(dir, WS)).toBeUndefined() + }) + + test("refuses to release a lock that was taken over", async () => { + const first = await acquire(dir, WS) + await rm(slot(), { recursive: true, force: true }) + const second = await acquire(dir, WS) + const token = (await meta()).token + + await first?.release() + expect((await meta()).token).toBe(token) + await second?.release() + }) + + test("reclaims a vanished lock directory instead of wedging", async () => { + const first = await acquire(dir, WS) + expect(first).toBeDefined() + // Winner reclaimed the stale directory out from under a racing claim: + // the loser must fall through to claiming, not report held-forever. + await rm(slot(), { recursive: true, force: true }) + const second = await acquire(dir, WS) + expect(second).toBeDefined() + await second?.release() + }) + + test("a stale release never deletes the current lock directory", async () => { + const first = await acquire(dir, WS) + expect(first).toBeDefined() + // The first owner's directory vanished (taken over); a second session now + // owns the slot. The first owner's release must leave it alone. + await rm(slot(), { recursive: true, force: true }) + const second = await acquire(dir, WS) + const token = (await meta()).token + + await first?.release() + expect((await meta()).token).toBe(token) + expect((await readdir(slot())).sort()).toEqual(["heartbeat", "meta.json"]) + await second?.release() + }) +})