fix(core): fail closed on zero inode file cache (QwenLM#8290)

* fix(core): fail closed on zero inode file cache

* chore(core): address zero inode review suggestions

* fix(core): reject zero inode transcript identity

* fix(core): distinguish unverifiable file identity

* fix(core): keep zero-inode fallout out of transcript pagination

Addresses the review on QwenLM#8290.

- reader: restore the plain inode comparison and record why the
  transcript reader does not need the fail-closed treatment. It only
  compares one session's path against itself and the cursor already
  carries a content-derived proof, whereas the cache and the lease
  compare identities that can belong to different files. Refusing zero
  there made readPage reject the very cursor it had just issued, so
  pagination died past page one.
- lease: probe the transcript filesystem during acquire so a brand-new
  session on such a filesystem fails up front instead of going quiet at
  the first append.
- share hasVerifiableInode() across the three call sites and handle the
  bigint stat shape.
- cover the PRIOR_READ_VERIFICATION_FAILED branches in edit, write-file
  and notebook-edit.

---------

Co-authored-by: 冼健聪 <mark.xian@evenrealities.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>

Author: 3 people

packages/core/src/services/fileReadCache.test.ts

@@ -38,6 +38,24 @@ describe('FileReadCache', () => {
       const b = FileReadCache.inodeKey(makeStats({ dev: 2, ino: 1 }));
       expect(a).not.toBe(b);
     });
+
+    it('treats ino 0 as unverifiable identity', () => {
+      expect(FileReadCache.hasVerifiableIdentity(makeStats({ ino: 0 }))).toBe(
+        false,
+      );
+      expect(FileReadCache.hasVerifiableIdentity(makeStats({ ino: 1 }))).toBe(
+        true,
+      );
+    });
+
+    it('treats a bigint ino 0 as unverifiable identity', () => {
+      // `stat(..., { bigint: true })` is used elsewhere in the repo, and
+      // `0n !== 0` is true, so the check must not be a raw `!==`.
+      const bigintStats = makeStats({
+        ino: 0n as unknown as number,
+      });
+      expect(FileReadCache.hasVerifiableIdentity(bigintStats)).toBe(false);
+    });
   });
 
   describe('check', () => {
@@ -87,6 +105,19 @@ describe('FileReadCache', () => {
       expect(cache.check(makeStats({ ino: 200 })).state).toBe('unknown');
     });
 
+    it('returns unverifiable for ino 0 even after a read was recorded', () => {
+      const cache = new FileReadCache();
+      const stats = makeStats({ dev: 7, ino: 0 });
+      const entry = cache.recordRead('/x/foo.ts', stats, {
+        full: true,
+        cacheable: true,
+      });
+
+      expect(entry.inodeKey).toBe('7:0');
+      expect(cache.size()).toBe(0);
+      expect(cache.check(stats).state).toBe('unverifiable');
+    });
+
     it('attaches the entry on fresh and stale results', () => {
       const cache = new FileReadCache();
       cache.recordRead('/x/foo.ts', makeStats(), {
@@ -125,6 +156,33 @@ describe('FileReadCache', () => {
       expect(entry.lastReadAt).toBe(Date.now());
     });
 
+    it('does not let ino 0 reads collide across paths', () => {
+      const cache = new FileReadCache();
+      const first = makeStats({ dev: 9, ino: 0, size: 10 });
+      const second = makeStats({ dev: 9, ino: 0, size: 20 });
+
+      cache.recordRead('/x/a.ts', first, { full: true, cacheable: true });
+      cache.recordRead('/x/b.ts', second, { full: true, cacheable: true });
+
+      expect(cache.size()).toBe(0);
+      expect(cache.check(first).state).toBe('unverifiable');
+      expect(cache.check(second).state).toBe('unverifiable');
+    });
+
+    it('returns a detached entry for an ino 0 read', () => {
+      const cache = new FileReadCache();
+      const stats = makeStats({ dev: 9, ino: 0 });
+
+      const entry = cache.recordRead('/x/a.ts', stats, {
+        full: true,
+        cacheable: true,
+      });
+      entry.readResidentInHistory = true;
+
+      expect(cache.size()).toBe(0);
+      expect(cache.check(stats).state).toBe('unverifiable');
+    });
+
     it('preserves full vs ranged read distinction', () => {
       const cache = new FileReadCache();
       const stats = makeStats();
@@ -262,6 +320,17 @@ describe('FileReadCache', () => {
       expect(entry.lastWriteAt).toBe(Date.now());
     });
 
+    it('does not cache writes with ino 0', () => {
+      const cache = new FileReadCache();
+      const stats = makeStats({ dev: 7, ino: 0 });
+
+      const entry = cache.recordWrite('/x/foo.ts', stats);
+
+      expect(entry.lastWriteAt).toBe(Date.now());
+      expect(cache.size()).toBe(0);
+      expect(cache.check(stats).state).toBe('unverifiable');
+    });
+
     it('seeds read metadata when recording a write on a brand-new entry', () => {
       // The model authored the bytes it just wrote — for the purposes
       // of prior-read enforcement on the *next* Edit, that counts as
@@ -679,25 +748,25 @@ describe('FileReadCache', () => {
     it('evicts the oldest entry when the cache exceeds MAX_ENTRIES', () => {
       // Fill cache to capacity (MAX_ENTRIES = 4096).
       const cache = new FileReadCache();
-      for (let i = 0; i < 4096; i++) {
+      for (let i = 1; i <= 4096; i++) {
         cache.recordRead(`/x/file-${i}.ts`, makeStats({ ino: i }), {
           full: true,
           cacheable: true,
         });
       }
       expect(cache.size()).toBe(4096);
 
-      // The 4097th write triggers eviction of the oldest (ino=0).
-      cache.recordWrite('/x/file-new.ts', makeStats({ ino: 4096 }));
+      // The 4097th write triggers eviction of the oldest (ino=1).
+      cache.recordWrite('/x/file-new.ts', makeStats({ ino: 4097 }));
       expect(cache.size()).toBeLessThanOrEqual(4096);
-      expect(cache.check(makeStats({ ino: 0 })).state).toBe('unknown');
-      expect(cache.check(makeStats({ ino: 4096 })).state).toBe('fresh');
+      expect(cache.check(makeStats({ ino: 1 })).state).toBe('unknown');
+      expect(cache.check(makeStats({ ino: 4097 })).state).toBe('fresh');
     });
 
     it('keeps size at MAX_ENTRIES after multiple overflows', () => {
       const cache = new FileReadCache();
       // Add MAX_ENTRIES + 100 distinct inodes.
-      for (let i = 0; i < 4196; i++) {
+      for (let i = 1; i <= 4196; i++) {
         cache.recordRead(`/x/file-${i}.ts`, makeStats({ ino: i }), {
           full: true,
           cacheable: true,
@@ -709,17 +778,17 @@ describe('FileReadCache', () => {
     it('should have bumped entries survive eviction', () => {
       const cache = new FileReadCache();
       // Fill to capacity.
-      for (let i = 0; i < 4096; i++) {
+      for (let i = 1; i <= 4096; i++) {
         cache.recordRead(`/x/file-${i}.ts`, makeStats({ ino: i }), {
           full: true,
           cacheable: true,
         });
       }
 
-      // Frequently update ino=0 — after bump lands this moves it to the
+      // Frequently update ino=1 — after bump lands this moves it to the
       // back of the eviction queue.
       for (let i = 0; i < 10; i++) {
-        cache.recordRead('/x/file-0.ts', makeStats({ ino: 0 }), {
+        cache.recordRead('/x/file-1.ts', makeStats({ ino: 1 }), {
           full: true,
           cacheable: true,
         });
@@ -734,7 +803,7 @@ describe('FileReadCache', () => {
       }
 
       expect(cache.size()).toBeLessThanOrEqual(4096);
-      expect(cache.check(makeStats({ ino: 0 })).state).not.toBe('unknown');
+      expect(cache.check(makeStats({ ino: 1 })).state).not.toBe('unknown');
     });
   });
 
@@ -812,7 +881,7 @@ describe('FileReadCache', () => {
       vi.setSystemTime(now);
 
       // 3 recent entries
-      for (let i = 0; i < 3; i++) {
+      for (let i = 1; i <= 3; i++) {
         cache.recordRead(`/x/recent-${i}.ts`, makeStats({ ino: i }), {
           full: true,
           cacheable: true,

packages/core/src/services/fileReadCache.ts

@@ -6,6 +6,7 @@
 
 import type { Stats } from 'node:fs';
 import { resolve as resolvePath } from 'node:path';
+import { hasVerifiableInode } from '../utils/file-identity.js';
 
 /**
  * Session-scoped cache that tracks which files the model has Read or
@@ -25,11 +26,23 @@ import { resolve as resolvePath } from 'node:path';
  * case-insensitive filesystems all collapse onto the same entry, which
  * is what we want — the cache is reasoning about *files*, not strings.
  *
- * Platform note: on Windows, `Stats.ino` is documented as not guaranteed
- * unique (Node returns it from `_BY_HANDLE_FILE_INFORMATION.nFileIndex`,
- * which can collide across volumes and ReFS). Callers that target
- * Windows should consider falling back to a path-based key; the POSIX
- * platforms qwen-code primarily runs on (macOS / Linux) are unaffected.
+ * Platform note: when `Stats.ino` is `0` (seen on FAT/exFAT and some
+ * SMB-style filesystems), inode identity is unverifiable. The cache
+ * deliberately does not store those reads/writes, so later mutation
+ * checks fail closed instead of treating unrelated files as the same
+ * `dev:0` entry. On Windows, non-zero `ino` values from `nFileIndex`
+ * can also collide across volumes and on ReFS; a path-based key
+ * fallback for that case is not yet implemented.
+ *
+ * Keying on the resolved path when `ino === 0` would keep Edit /
+ * WriteFile usable on those filesystems while still giving distinct
+ * paths distinct entries, and it is the obvious follow-up if anyone
+ * reports the loss. It is deliberately not done here: a path key is
+ * strictly weaker (an in-place replacement that preserves mtime and
+ * size reads as the same file, and FAT's 2-second mtime granularity
+ * makes that collision cheap), so it trades a silent wrong edit for
+ * an availability win. This cache chose the honest failure first;
+ * availability can be added later behind an explicit decision.
  *
  * Lifecycle: one instance is created per `Config` via the field
  * initializer, so any code that constructs its own Config — notably
@@ -131,6 +144,7 @@ export interface FileReadEntry {
 export type FileReadCheckResult =
   | { state: 'fresh'; entry: FileReadEntry }
   | { state: 'stale'; entry: FileReadEntry }
+  | { state: 'unverifiable' }
   | { state: 'unknown' };
 
 export class FileReadCache {
@@ -142,6 +156,11 @@ export class FileReadCache {
     return `${stats.dev}:${stats.ino}`;
   }
 
+  /** See {@link hasVerifiableInode}. */
+  static hasVerifiableIdentity(stats: Stats): boolean {
+    return hasVerifiableInode(stats.ino);
+  }
+
   /**
    * Record a successful Read of `absPath`.
    *
@@ -180,12 +199,26 @@ export class FileReadCache {
    * The fast-path `file_unchanged` check still gates on the
    * incoming request's own `isFullRead` (in `read-file.ts`), so a
    * partial read does not get a placeholder it shouldn't.
+   *
+   * When `stats.ino` is `0` the read is not stored and the returned
+   * entry is **detached**: it describes this read for the immediate
+   * caller, but it is not in the map, so mutating it has no effect
+   * and a later {@link check} still reports `unverifiable`.
    */
   recordRead(
     absPath: string,
     stats: Stats,
     opts: { full: boolean; cacheable: boolean },
   ): FileReadEntry {
+    if (!FileReadCache.hasVerifiableIdentity(stats)) {
+      const entry = FileReadCache.createEntry(absPath, stats);
+      entry.lastReadAt = Date.now();
+      entry.readResidentInHistory = opts.full;
+      entry.lastReadWasFull = opts.full;
+      entry.lastReadCacheable = opts.cacheable;
+      return entry;
+    }
+
     const key = FileReadCache.inodeKey(stats);
     const existing = this.byInode.get(key);
     const sameFingerprint =
@@ -238,13 +271,18 @@ export class FileReadCache {
    * the default `cacheable: true`; structured writers such as notebook cell
    * editors can set `cacheable: false` so regular Edit / WriteFile still
    * reject the file as a non-text payload.
+   *
+   * As with {@link recordRead}, an `ino === 0` write returns a
+   * **detached** entry that was never added to the map.
    */
   recordWrite(
     absPath: string,
     stats: Stats,
     opts: { cacheable?: boolean } = {},
   ): FileReadEntry {
-    const entry = this.upsert(absPath, stats);
+    const entry = FileReadCache.hasVerifiableIdentity(stats)
+      ? this.upsert(absPath, stats)
+      : FileReadCache.createEntry(absPath, stats);
     const now = Date.now();
     entry.lastWriteAt = now;
     entry.lastReadAt = now;
@@ -259,6 +297,8 @@ export class FileReadCache {
   /**
    * Compare the cached fingerprint against `stats` for the same inode.
    *
+   *  - `unverifiable` — the filesystem reported `ino === 0`, so the
+   *    file identity cannot be safely compared or cached.
    *  - `unknown` — no entry. The file has never been Read or written in
    *    this session.
    *  - `stale`   — entry exists but mtime or size differs. The file has
@@ -273,6 +313,10 @@ export class FileReadCache {
    * `0 occurrences` failure mode, which prompts the model to re-read.
    */
   check(stats: Stats): FileReadCheckResult {
+    if (!FileReadCache.hasVerifiableIdentity(stats)) {
+      return { state: 'unverifiable' };
+    }
+
     const entry = this.byInode.get(FileReadCache.inodeKey(stats));
     if (!entry) return { state: 'unknown' };
     if (entry.mtimeMs !== stats.mtimeMs || entry.sizeBytes !== stats.size) {
@@ -301,6 +345,10 @@ export class FileReadCache {
    * fall back to {@link clear}.
    */
   markReadEvictedFromHistory(stats: Stats): boolean {
+    if (!FileReadCache.hasVerifiableIdentity(stats)) {
+      return false;
+    }
+
     const entry = this.byInode.get(FileReadCache.inodeKey(stats));
     if (entry) {
       entry.readResidentInHistory = false;
@@ -311,6 +359,10 @@ export class FileReadCache {
 
   /** Remove the entry for the given Stats, if any. */
   invalidate(stats: Stats): void {
+    if (!FileReadCache.hasVerifiableIdentity(stats)) {
+      return;
+    }
+
     this.byInode.delete(FileReadCache.inodeKey(stats));
   }
 
@@ -397,16 +449,20 @@ export class FileReadCache {
         this.byInode.delete(oldestKey);
       }
     }
-    const entry: FileReadEntry = {
-      inodeKey: key,
+    const entry = FileReadCache.createEntry(absPath, stats);
+    this.byInode.set(key, entry);
+    return entry;
+  }
+
+  private static createEntry(absPath: string, stats: Stats): FileReadEntry {
+    return {
+      inodeKey: FileReadCache.inodeKey(stats),
       realPath: absPath,
       mtimeMs: stats.mtimeMs,
       sizeBytes: stats.size,
       lastReadWasFull: false,
       lastReadCacheable: false,
       readResidentInHistory: false,
     };
-    this.byInode.set(key, entry);
-    return entry;
   }
 }