fix(watcher): ignore Windows access-only notifications (#1451) (#2050)

* fix Windows watcher false edits from NTFS atime * fix(watcher): ignore Windows access-only notifications (#1451) Check indexed size and mtime before recording Windows watcher edits. Preserve scope reconciliation and fail open when metadata cannot be verified. Add native Windows regression coverage and validate on Windows, macOS, and Linux. Co-authored-by: JJordan0K <69581081+JJordan0C@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: JJordan0K <69581081+JJordan0C@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-28 14:08:56 +08:00 · 2026-09-27 15:10:46 +00:00
parent 346d3e3702
commit 1b6be1c7d3
3 changed files with 216 additions and 4 deletions
@@ -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<typeof import('fs')>() }));
+
 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<string>) =>
+        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<typeof fs.statSync>) => {
+            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)', () => {
@@ -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();
@@ -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();