Repository navigation
fix(indexing): replace chokidar file watcher with @parcel/watcher #12784
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
61ec24d
07b082e
0a12bda
3b4cc06
f15f88b
1d267c6
9d83843
c4834a1
d9c94b7
1da932b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,16 @@ | ||
| import { watch as chokidarWatch, type FSWatcher as ChokidarFSWatcher } from "chokidar" | ||
| import type ParcelWatcher from "@parcel/watcher" | ||
| import { stat, readFile } from "fs/promises" | ||
| import { createHash } from "crypto" | ||
| import path from "path" | ||
| import { v5 as uuidv5 } from "uuid" | ||
| import { Emitter, type Disposable } from "../runtime" | ||
| import { Emitter } from "../runtime" | ||
| import { | ||
| QDRANT_CODE_BLOCK_NAMESPACE, | ||
| MAX_FILE_SIZE_BYTES, | ||
| BATCH_SEGMENT_THRESHOLD, | ||
| MAX_BATCH_RETRIES, | ||
| INITIAL_RETRY_DELAY_MS, | ||
| PARCEL_SUBSCRIBE_TIMEOUT_MS, | ||
| } from "../constants" | ||
| import { scannerExtensions } from "../shared/supported-extensions" | ||
| import { | ||
|
|
@@ -38,15 +39,107 @@ import { isBinary } from "../shared/is-binary" | |
|
|
||
| const log = Log.create({ service: "file-watcher" }) | ||
|
|
||
| /** | ||
| * Implementation of the file watcher interface. | ||
| * | ||
| * RATIONALE: Uses chokidar instead of vscode.workspace.createFileSystemWatcher | ||
| * so the watcher works outside VS Code (CLI, tests, headless). | ||
| */ | ||
| declare const KILO_LIBC: string | undefined | ||
|
|
||
| export interface FileWatchEvent { | ||
| path: string | ||
| type: "create" | "update" | "delete" | ||
| } | ||
|
|
||
| export interface FileWatchSubscription { | ||
| unsubscribe(): Promise<void> | ||
| } | ||
|
|
||
| // Injectable so tests can supply a fake watcher instead of a real subscription. | ||
| export type FileWatchSubscribe = ( | ||
| directory: string, | ||
| onEvents: (events: readonly FileWatchEvent[]) => void, | ||
| ) => Promise<FileWatchSubscription> | ||
|
|
||
| function watcherBackend(): ParcelWatcher.BackendType | undefined { | ||
| if (process.platform === "win32") return "windows" | ||
| if (process.platform === "darwin") return "fs-events" | ||
| if (process.platform === "linux") return "inotify" | ||
| return undefined | ||
| } | ||
|
|
||
| let parcelModule: typeof import("@parcel/watcher") | null | undefined | ||
| function loadParcelWatcher(): typeof import("@parcel/watcher") | undefined { | ||
| if (parcelModule !== undefined) return parcelModule ?? undefined | ||
| try { | ||
| // Load the platform binding directly; this is what resolves inside the | ||
| // bundled bun-compiled runtime (mirrors @kilocode/core's workspace watcher). | ||
| const libc = typeof KILO_LIBC === "undefined" ? undefined : KILO_LIBC | ||
| const suffix = process.platform === "linux" ? `-${libc || "glibc"}` : "" | ||
| // @ts-ignore - the wrapper subpath ships without type declarations | ||
| const { createWrapper } = require("@parcel/watcher/wrapper") | ||
| const binding = require(`@parcel/watcher-${process.platform}-${process.arch}${suffix}`) | ||
| parcelModule = createWrapper(binding) as typeof import("@parcel/watcher") | ||
| } catch { | ||
| try { | ||
| parcelModule = require("@parcel/watcher") as typeof import("@parcel/watcher") // dev / CLI / test fallback | ||
| } catch { | ||
| parcelModule = null | ||
| } | ||
| } | ||
| return parcelModule ?? undefined | ||
| } | ||
|
|
||
| // Prune the largest excluded directories from the native watch. Per-repo | ||
| // .gitignore/.kilocodeignore are enforced in shouldIndex(), not pruned here. | ||
| function watcherIgnoreGlobs(): string[] { | ||
| return FileIgnore.PATTERNS.map((pattern) => (pattern.includes("/") ? pattern : `**/${pattern}`)) | ||
| } | ||
|
|
||
| function withSubscribeTimeout( | ||
| pending: Promise<FileWatchSubscription>, | ||
| directory: string, | ||
| ): Promise<FileWatchSubscription> { | ||
| let timedOut = false | ||
| let timer: NodeJS.Timeout | undefined | ||
| const timeout = new Promise<never>((_, reject) => { | ||
| timer = setTimeout(() => { | ||
| timedOut = true | ||
| reject(new Error(`Timed out establishing the file watcher subscription for "${directory}".`)) | ||
| }, PARCEL_SUBSCRIBE_TIMEOUT_MS) | ||
| timer.unref() | ||
| }) | ||
| // Tear down a subscription that only resolves after we have already given up. | ||
| void pending | ||
| .then((subscription) => (timedOut ? subscription.unsubscribe() : undefined)) | ||
| .catch((err) => { | ||
| if (timedOut) log.warn("failed to tear down late file watcher subscription", { err, workspacePath: directory }) | ||
| }) | ||
| return Promise.race([pending, timeout]).finally(() => { | ||
| if (timer) clearTimeout(timer) | ||
| }) | ||
| } | ||
|
|
||
| // @parcel/watcher subscribes without a blocking initial walk (reporting only | ||
| // future changes), so it does not stall startup the way chokidar did under bun. | ||
| const parcelSubscribe: FileWatchSubscribe = (directory, onEvents) => { | ||
| const parcel = loadParcelWatcher() | ||
| if (!parcel) { | ||
| return Promise.reject(new Error("native file watcher backend (@parcel/watcher) is unavailable")) | ||
| } | ||
| const pending = parcel.subscribe( | ||
| directory, | ||
| (error, events) => { | ||
| if (error) { | ||
| log.error("file watcher reported an error", { workspacePath: directory, err: error }) | ||
| return | ||
| } | ||
| onEvents(events) | ||
| }, | ||
| { ignore: watcherIgnoreGlobs(), backend: watcherBackend() }, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SUGGESTION: The The old chokidar With only So it isn't purely an efficiency prune as the doc comment on line 105 claims. Consider deriving extra globs from Reply with |
||
| ) | ||
| return withSubscribeTimeout(pending, directory) | ||
| } | ||
|
|
||
| /** Watches the workspace via @parcel/watcher and feeds changes into the incremental indexer. */ | ||
| export class FileWatcher implements IFileWatcher { | ||
| private ignoreInstance?: IgnoreMatcher | ||
| private watcher?: ChokidarFSWatcher | ||
| private subscription?: FileWatchSubscription | ||
| private accumulatedEvents: Map<string, { path: string; type: "create" | "change" | "delete" }> = new Map() | ||
| private batchProcessDebounceTimer?: NodeJS.Timeout | ||
| private readonly BATCH_DEBOUNCE_DELAY_MS = 500 | ||
|
|
@@ -68,6 +161,15 @@ export class FileWatcher implements IFileWatcher { | |
| }>() | ||
| public readonly onDidFinishBatchProcessing = new Emitter<BatchProcessingSummary>() | ||
|
|
||
| // @parcel/watcher reports "update" where the pipeline expects "change". | ||
| private readonly onWatchEvents = (events: readonly FileWatchEvent[]): void => { | ||
| for (const event of events) { | ||
| const type: "create" | "change" | "delete" = | ||
| event.type === "create" ? "create" : event.type === "delete" ? "delete" : "change" | ||
| this.handleFileEvent(event.path, type) | ||
| } | ||
| } | ||
|
|
||
| constructor( | ||
| private workspacePath: string, | ||
| private readonly cacheManager: CacheManager, | ||
|
|
@@ -80,6 +182,7 @@ export class FileWatcher implements IFileWatcher { | |
| private readonly telemetryMeta?: IndexingTelemetryMeta, | ||
| extensions: readonly string[] = scannerExtensions, | ||
| private readonly parser: ICodeParser = codeParser, | ||
| private readonly subscribeFn: FileWatchSubscribe = parcelSubscribe, | ||
| ) { | ||
| if (ignoreInstance) { | ||
| this.ignoreInstance = ignoreInstance | ||
|
|
@@ -124,10 +227,10 @@ export class FileWatcher implements IFileWatcher { | |
| } | ||
|
|
||
| /** | ||
| * Initializes the file watcher using chokidar. | ||
| * | ||
| * RATIONALE: chokidar watches the filesystem directly using native OS events, | ||
| * removing the dependency on VS Code's file system watcher API. | ||
| * Subscribes to the workspace for incremental updates. The watcher is only | ||
| * needed for incremental updates, so a subscription failure (missing native | ||
| * backend, inotify limits, ...) degrades to a warning and lets the full scan | ||
| * proceed rather than failing indexing. | ||
| */ | ||
| async initialize(): Promise<void> { | ||
| if (this.ready) { | ||
|
|
@@ -137,26 +240,18 @@ export class FileWatcher implements IFileWatcher { | |
|
|
||
| log.info("initializing file watcher", { workspacePath: this.workspacePath }) | ||
|
|
||
| this.watcher = chokidarWatch(this.workspacePath, { | ||
| ignored: (filePath: string) => { | ||
| const relativeFilePath = generateRelativeIgnorePath(filePath, this.workspacePath) | ||
| if (!relativeFilePath) return false | ||
| if (FileIgnore.match(relativeFilePath)) return true | ||
| return this.ignoreInstance?.ignores(relativeFilePath) ?? false | ||
| }, | ||
| persistent: true, | ||
| ignoreInitial: true, | ||
| }) | ||
|
|
||
| this.watcher.on("add", (filePath) => this.handleFileEvent(filePath, "create")) | ||
| this.watcher.on("change", (filePath) => this.handleFileEvent(filePath, "change")) | ||
| this.watcher.on("unlink", (filePath) => this.handleFileEvent(filePath, "delete")) | ||
| this.ready = new Promise((resolve, reject) => { | ||
| this.watcher?.once("ready", resolve) | ||
| this.watcher?.once("error", reject) | ||
| }) | ||
| this.ready = this.subscribeFn(this.workspacePath, this.onWatchEvents) | ||
| .then((subscription) => { | ||
| this.subscription = subscription | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WARNING: A subscription that resolves after Neither Consider capturing the pending promise ( Reply with |
||
| log.info("file watcher subscribed", { workspacePath: this.workspacePath }) | ||
| }) | ||
| .catch((error) => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SUGGESTION: Latching the failure into a resolved Because this Reply with |
||
| log.warn("file watcher unavailable; continuing without incremental updates", { | ||
| workspacePath: this.workspacePath, | ||
| err: error, | ||
| }) | ||
| }) | ||
| await this.ready | ||
| log.info("file watcher ready", { workspacePath: this.workspacePath }) | ||
| } | ||
|
|
||
| setOverlay(overlay?: WorktreeOverlay): void { | ||
|
|
@@ -191,14 +286,24 @@ export class FileWatcher implements IFileWatcher { | |
| this.collecting = false | ||
| if (this.batchProcessDebounceTimer) clearTimeout(this.batchProcessDebounceTimer) | ||
| this.batchProcessDebounceTimer = undefined | ||
| await this.watcher?.close() | ||
| const subscription = this.subscription | ||
| this.subscription = undefined | ||
| await subscription | ||
| ?.unsubscribe() | ||
| .catch((err) => log.error("failed to unsubscribe file watcher", { err, workspacePath: this.workspacePath })) | ||
| await this.drainTask | ||
| this.dispose() | ||
| } | ||
|
|
||
| dispose(): void { | ||
| this.collecting = false | ||
| void this.watcher?.close() | ||
| const subscription = this.subscription | ||
| this.subscription = undefined | ||
| if (subscription) { | ||
| void subscription | ||
| .unsubscribe() | ||
| .catch((err) => log.error("failed to unsubscribe file watcher", { err, workspacePath: this.workspacePath })) | ||
| } | ||
| if (this.batchProcessDebounceTimer) clearTimeout(this.batchProcessDebounceTimer) | ||
| this.batchProcessDebounceTimer = undefined | ||
| this.onDidStartBatchProcessing.dispose() | ||
|
|
@@ -209,7 +314,7 @@ export class FileWatcher implements IFileWatcher { | |
| } | ||
|
|
||
| /** | ||
| * Handles a file event from chokidar by accumulating it and scheduling batch processing. | ||
| * Handles a file event reported by the watcher by accumulating it and scheduling batch processing. | ||
| */ | ||
| private handleFileEvent(filePath: string, type: "create" | "change" | "delete"): void { | ||
| if (!this.shouldIndex(filePath)) return | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SUGGESTION: A late rejection of
pendingis logged as a teardown failure, mislabeling the root causeThis
.catchhandles two different errors: (a)unsubscribe()failing while tearing down a late-arriving subscription, and (b)pendingitself rejecting after the timeout already fired. In case (b) the subscribe failed — there was never anything to tear down — yet it is logged as "failed to tear down late file watcher subscription", whileinitialize()already logged the timeout as the failure reason. The real error is preserved inerr, but the message points at the wrong operation; consider distinguishing the cases (e.g. a rejection handler on.thenthat logs "file watcher subscribe failed after the timeout").Reply with
@kilocode-bot fix itto have Kilo Code address this issue.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kilocode-bot fix it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To use Kilo from GitHub you first need to link your GitHub account to Kilo. Link your Kilo account to continue. After linking, mention me again in this issue or pull request.