diff --git a/__tests__/watcher.test.ts b/__tests__/watcher.test.ts index 6af1467a8..05349c430 100644 --- a/__tests__/watcher.test.ts +++ b/__tests__/watcher.test.ts @@ -19,6 +19,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { EventEmitter } from 'events'; +import { execFileSync } from 'child_process'; import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; @@ -31,6 +32,9 @@ import { } from '../src/sync/watcher'; import CodeGraph from '../src/index'; +// Keep real filesystem operations, with writable exports for failure injection. +vi.mock('fs', async (importOriginal) => ({ ...await importOriginal() })); + type SyncFn = (paths?: string[]) => Promise<{ filesChanged: number; durationMs: number }>; /** @@ -67,8 +71,12 @@ describe('FileWatcher', () => { // Inert by default — unit tests drive events via __emitWatchEventForTests // and never depend on real OS watch delivery. - const newWatcher = (syncFn: SyncFn, opts: WatchOptions = {}) => { - const watcher = new FileWatcher(testDir, syncFn, { inertForTests: true, ...opts }); + const newWatcher = ( + syncFn: SyncFn, + opts: WatchOptions = {}, + isFileStateCurrent?: (relativePath: string) => boolean + ) => { + const watcher = new FileWatcher(testDir, syncFn, { inertForTests: true, ...opts }, isFileStateCurrent); unitWatchers.add(watcher); return watcher; }; @@ -779,6 +787,47 @@ describe('FileWatcher', () => { }); describe('pending file tracking (#403)', () => { + it('should ignore events whose filesystem metadata is already indexed (#1451)', async () => { + const syncFn = vi.fn().mockResolvedValue({ filesChanged: 0, durationMs: 0 }); + const isFileStateCurrent = vi.fn().mockReturnValue(true); + const watcher = newWatcher( + syncFn, + { debounceMs: 100 }, + isFileStateCurrent + ); + watcher.start(); + await watcher.waitUntilReady(); + + __emitWatchEventForTests(testDir, 'src/index.ts'); + + expect(isFileStateCurrent).toHaveBeenCalledWith('src/index.ts'); + expect(watcher.getPendingFiles()).toEqual([]); + await new Promise((r) => setTimeout(r, 200)); + expect(syncFn).not.toHaveBeenCalled(); + + watcher.stop(); + }); + + it('should keep an event when the metadata check fails open (#1451)', async () => { + const syncFn = vi.fn().mockResolvedValue({ filesChanged: 1, durationMs: 10 }); + const isFileStateCurrent = vi.fn(() => { + throw new Error('database busy'); + }); + const watcher = newWatcher( + syncFn, + { debounceMs: 2000 }, + isFileStateCurrent + ); + watcher.start(); + await watcher.waitUntilReady(); + + __emitWatchEventForTests(testDir, 'src/index.ts'); + + expect(watcher.getPendingFiles().map((p) => p.path)).toContain('src/index.ts'); + + watcher.stop(); + }); + it('should expose edited paths via getPendingFiles before sync fires', async () => { // Slow debounce — pending entries are visible until the debounce fires. // The synthetic event is synchronous, so we can assert immediately. @@ -999,6 +1048,116 @@ describe('FileWatcher', () => { cg.unwatch(); }); + it.runIf(process.platform === 'win32')( + 'should ignore an NTFS access-only event but retain a real edit (#1451)', + async () => { + const filePath = path.join(testDir, 'src', 'index.ts'); + cg = initGraph(testDir, { + config: { include: ['**/*.ts'], exclude: [] }, + }); + await cg.indexAll(); + + cg.watch({ debounceMs: 2000, inertForTests: true }); + await cg.waitUntilWatcherReady(); + + const before = fs.statSync(filePath); + fs.utimesSync( + filePath, + new Date(before.atimeMs + 2000), + new Date(before.mtimeMs) + ); + __emitWatchEventForTests(testDir, 'src/index.ts'); + + expect(cg.getPendingFiles()).toEqual([]); + + fs.appendFileSync(filePath, '\nexport const changed = true;\n'); + __emitWatchEventForTests(testDir, 'src/index.ts'); + + expect(cg.getPendingFiles().map((p) => p.path)).toContain('src/index.ts'); + + cg.unwatch(); + } + ); + }); + + describe('Windows indexed metadata guard (#1451)', () => { + it.runIf(process.platform === 'win32')('ignores native atime and separate-process read events, then reconciles writes and deletion', async () => { + const file = path.join(testDir, 'src', 'index.ts'); + const cg = initGraph(testDir, { config: { include: ['**/*.ts'], exclude: [] } }); + await cg.indexAll(); + const synced = vi.fn(); + // Observe real OS delivery through the same stream CodeGraph consumes. + // No injected events: a passing negative assertion must see a native event. + const nativeWatch = fs.watch; + let events = 0; + __setFsWatchForTests(((dir: fs.PathLike, options: fs.WatchOptions, listener: fs.WatchListener) => + nativeWatch(dir, options, (event, filename) => { + listener(event, filename); + if (String(filename).replace(/\\/g, '/') === 'src/index.ts') events++; + })) as typeof fs.watch); + expect(cg.watch({ debounceMs: 2000, onSyncComplete: synced })).toBe(true); + await cg.waitUntilWatcherReady(); + const before = fs.statSync(file); + const quotedFile = file.replace(/'/g, "''"); + execFileSync('powershell.exe', ['-NoProfile', '-Command', + `(Get-Item -LiteralPath '${quotedFile}').LastAccessTimeUtc = [DateTime]::UtcNow.AddDays(-2)`]); + await waitFor(() => events > 0, 5000); + expect(fs.statSync(file).mtimeMs).toBe(before.mtimeMs); + expect(fs.statSync(file).size).toBe(before.size); + expect(cg.getPendingFiles()).toEqual([]); + events = 0; + execFileSync(process.execPath, ['-e', 'require("fs").readFileSync(process.argv[1])', file]); + // NTFS can delay/coalesce automatic last-access updates. The explicit + // atime event above guarantees native coverage even on such volumes. + await new Promise(resolve => setTimeout(resolve, 250)); + expect(fs.statSync(file).mtimeMs).toBe(before.mtimeMs); + expect(fs.statSync(file).size).toBe(before.size); + expect(cg.getPendingFiles()).toEqual([]); + expect(synced).not.toHaveBeenCalled(); + + fs.appendFileSync(file, '\nexport const nativeEdit1451 = true;\n'); + await waitFor(() => cg.getPendingFiles().some(p => p.path === 'src/index.ts')); + await waitFor(() => cg.getNodesByName('nativeEdit1451').length > 0 && !cg.isIndexing(), 8000); + await waitFor(() => cg.getPendingFiles().length === 0); + fs.unlinkSync(file); + await waitFor(() => cg.getPendingFiles().some(p => p.path === 'src/index.ts')); + await waitFor(() => cg.getNodesByName('nativeEdit1451').length === 0 && !cg.isIndexing(), 8000); + }, 30000); + + it.runIf(process.platform === 'win32').each(['unknown', 'deleted', 'inaccessible', 'unverifiable', 'mtime-only'])( + 'keeps %s files pending', async (state) => { + const file = path.join(testDir, 'src', 'index.ts'); + const cg = initGraph(testDir, { config: { include: ['**/*.ts'], exclude: [] } }); + await cg.indexAll(); + cg.watch({ debounceMs: 10000, inertForTests: true }); + await cg.waitUntilWatcherReady(); + let relative = 'src/index.ts'; + if (state === 'unknown') { + relative = 'src/new.ts'; + fs.writeFileSync(path.join(testDir, relative), 'export const newFile = 1;'); + } else if (state === 'deleted') { + fs.unlinkSync(file); + } else if (state === 'mtime-only') { + const stat = fs.statSync(file); + fs.writeFileSync(file, 'export const x = 2;'); + fs.utimesSync(file, stat.atime, new Date(stat.mtimeMs + 2000)); + } else { + const statSync = fs.statSync; + vi.spyOn(fs, 'statSync').mockImplementation(((...args: Parameters) => { + if (String(args[0]) === file) { + if (state === 'inaccessible') throw Object.assign(new Error('access denied'), { code: 'EACCES' }); + const stat = statSync(file); + stat.mtimeMs = NaN; + return stat; + } + return statSync(...args); + }) as typeof fs.statSync); + } + __emitWatchEventForTests(testDir, relative); + expect(cg.getPendingFiles().map(p => p.path)).toContain(relative); + vi.restoreAllMocks(); + } + ); }); describe('symlink directory watching (#770)', () => { @@ -1153,6 +1312,7 @@ describe('FileWatcher', () => { expect(watcher.isDegraded()).toBe(true); expect(close).toHaveBeenCalledTimes(3); }); + }); describe('scoped sync fast path (#watcher-scoped)', () => { diff --git a/src/index.ts b/src/index.ts index 266e71607..df7974351 100644 --- a/src/index.ts +++ b/src/index.ts @@ -5,6 +5,7 @@ * knowledge graph from any codebase. */ +import * as fs from 'fs'; import * as path from 'path'; import { Node, @@ -1101,6 +1102,31 @@ export class CodeGraph { // File Watching // =========================================================================== + /** + * Whether an OS watcher event points at a file whose index metadata is still + * current. Used on Windows to discard NTFS last-access notifications, which + * libuv reports through fs.watch as ordinary change events (#1451). + */ + private isIndexedFileStateCurrent(filePath: string): boolean { + const tracked = this.queries.getFileByPath(filePath); + if (!tracked) return false; + + try { + const stat = fs.statSync(path.join(this.projectRoot, filePath)); + return ( + stat.isFile() && + Number.isFinite(stat.mtimeMs) && + Number.isFinite(tracked.modifiedAt) && + stat.size === tracked.size && + Math.floor(stat.mtimeMs) === Math.floor(tracked.modifiedAt) + ); + } catch { + // Missing/inaccessible files must still reach sync so removals and + // transient filesystem failures are reconciled rather than hidden. + return false; + } + } + /** * Start watching for file changes and auto-syncing. * @@ -1120,7 +1146,10 @@ export class CodeGraph { const filesChanged = result.filesAdded + result.filesModified + result.filesRemoved; return { filesChanged, durationMs: result.durationMs }; }, - options + options, + process.platform === 'win32' + ? (filePath) => this.isIndexedFileStateCurrent(filePath) + : undefined ); return this.watcher.start(); diff --git a/src/sync/watcher.ts b/src/sync/watcher.ts index 0d1b13148..17722a8fb 100644 --- a/src/sync/watcher.ts +++ b/src/sync/watcher.ts @@ -363,11 +363,19 @@ export class FileWatcher { private readonly onSyncError?: WatchOptions['onSyncError']; private readonly onDegraded?: WatchOptions['onDegraded']; private readonly inertForTests: boolean; + /** + * Optional metadata guard supplied by the CodeGraph facade. Windows' + * ReadDirectoryChangesW stream includes last-access updates, so a read can + * otherwise look exactly like an edit. Returning true means the file's + * current size/mtime still match the indexed record and the event is noise. + */ + private readonly isFileStateCurrent?: (relativePath: string) => boolean; constructor( projectRoot: string, syncFn: (paths?: string[]) => Promise<{ filesChanged: number; durationMs: number }>, - options: WatchOptions = {} + options: WatchOptions = {}, + isFileStateCurrent?: (relativePath: string) => boolean ) { this.projectRoot = projectRoot; this.syncFn = syncFn; @@ -376,6 +384,7 @@ export class FileWatcher { this.onSyncError = options.onSyncError; this.onDegraded = options.onDegraded; this.inertForTests = options.inertForTests ?? false; + this.isFileStateCurrent = isFileStateCurrent; } /** @@ -646,6 +655,20 @@ export class FileWatcher { return; } + try { + if (this.isFileStateCurrent?.(rel)) { + logDebug('Ignoring file event with unchanged indexed metadata', { file: rel }); + return; + } + } catch (error) { + // The guard is an optimization, never a correctness boundary. If the + // stat/DB lookup fails, keep the event so a sync can reconcile it. + logDebug('Could not verify file event metadata; treating it as a change', { + file: rel, + error: String(error), + }); + } + logDebug('File change detected', { file: rel }); if (this.ready) { const now = Date.now();