fix(graph): invalidate cached context after atomic file replacement

src/graph/load.ts

@@ -1,23 +1,20 @@
 /**
- * mtime-keyed in-process cache over the two readers `ask()` calls on every
+ * File-metadata-keyed in-process cache over the two readers `ask()` calls on every
  * query: the wiring graph (`readGraph`) and the ask sidecar (`readAskIndex`).
  * `graft ask` re-parses these from disk on every invocation; in a long-lived
  * process — the MCP server, or `graft ask` invoked repeatedly in one process —
  * that means re-parsing the same ~tens-of-MB JSON on every tool call.
  *
- * Keyed by `(path, mtimeMs, size)` from `statSync`, so a rebuild (`graft
- * build`) is picked up on the very next call with no polling and no TTL: the
- * stat is cheap relative to the parse it guards, and a changed mtime or size
- * invalidates the entry. A missing file returns null and is never cached — the
- * next call re-stats, so a file created after a prior miss is picked up
- * immediately (no negative caching).
+ * Keyed by `(path, dev, ino, ctimeMs, mtimeMs, size)` from `statSync`.
+ * Device/inode detect an atomic replacement even when its mtime and size are
+ * preserved; ctime, mtime and size detect observable in-place changes. A
+ * missing file returns null and drops any prior entry, so a file created after
+ * a miss is picked up on the next call (no negative caching).
  *
- * **Cache invalidation assumption:** This strategy assumes filesystem mtime
- * resolution is finer than build cadence. This is safe because `graft build`
- * rewrites the entire output file atomically, so same-size rewrites within a
- * single mtime tick (which would serve stale data) are infeasible in practice.
- * On APFS (macOS) mtime is nanosecond-granular and a build takes milliseconds,
- * so invalidation is immediate and reliable.
+ * Stat metadata is not a content hash: an in-place rewrite whose entire
+ * observed signature remains unchanged can still look like a warm entry.
+ * Same-process rebuild callers therefore continue to invalidate explicitly
+ * rather than depending on filesystem timestamp resolution.
  *
  * Dependency-free by design: this module imports only `node:fs`, `./write.js`,
  * and `../ask/index-file.js`, so it can be imported from both `ask.ts` and
@@ -28,9 +25,15 @@ import { readGraph, wiringPath } from "./write.js";
 import { readAskIndex, askIndexPath, type AskIndex } from "../ask/index-file.js";
 import type { GraphV1 } from "./types.js";
 
-interface CacheEntry<T> {
+interface FileStamp {
+  dev: number;
+  ino: number;
+  ctimeMs: number;
   mtimeMs: number;
   size: number;
+}
+
+interface CacheEntry<T> extends FileStamp {
   value: T | null;
 }
 
@@ -47,10 +50,10 @@ export function __resetParseCounts(): void {
   __parseCount.askIndex = 0;
 }
 
-function statOf(path: string): { mtimeMs: number; size: number } | null {
+function statOf(path: string): FileStamp | null {
   try {
     const s = statSync(path);
-    return { mtimeMs: s.mtimeMs, size: s.size };
+    return { dev: s.dev, ino: s.ino, ctimeMs: s.ctimeMs, mtimeMs: s.mtimeMs, size: s.size };
   } catch {
     return null;
   }
@@ -66,22 +69,29 @@ function loadCached<T>(
   if (!st) {
     // Missing file: don't negatively cache, and drop any stale entry so a
     // subsequently-created file at the same path is re-parsed, not served
-    // from a cache keyed to the old (mtime, size).
+    // from a cache keyed to the old file identity/metadata.
     cache.delete(path);
     return null;
   }
   const cached = cache.get(path);
-  if (cached && cached.mtimeMs === st.mtimeMs && cached.size === st.size) {
+  if (
+    cached &&
+    cached.dev === st.dev &&
+    cached.ino === st.ino &&
+    cached.ctimeMs === st.ctimeMs &&
+    cached.mtimeMs === st.mtimeMs &&
+    cached.size === st.size
+  ) {
     return cached.value;
   }
   onParse();
   const value = parse();
-  cache.set(path, { mtimeMs: st.mtimeMs, size: st.size, value });
+  cache.set(path, { ...st, value });
   return value;
 }
 
 /** Cached `readGraph(wiringPath(outDir))` — same null-on-missing/unparseable
- * semantics, re-reads only when the wiring file's `(mtimeMs, size)` changed.
+ * semantics, re-reads only when the wiring file's identity/metadata changed.
  * Returns a shared cached reference; callers must not mutate the returned graph. */
 export function loadGraphCached(outDir: string): GraphV1 | null {
   const path = wiringPath(outDir);
@@ -102,12 +112,10 @@ export function loadAskIndexCached(outDir: string): AskIndex | null {
 /**
  * Drop the cached graph + sidecar for `outDir`, forcing the next load to re-read.
  *
- * The `(mtimeMs, size)` key above is a good invalidator for a build that happened
- * in *another* process, but not for one this process just performed: the
- * pre-query auto-refresh (`refresh.ts`) rewrites the graph and then immediately
- * queries it, and on a filesystem with coarse mtime resolution a same-tick rewrite
- * of the same size would be served from the stale entry. Callers that rebuild must
- * therefore invalidate explicitly rather than trust the clock.
+ * The file identity/metadata key detects external atomic replacements, but
+ * cannot guarantee detection of an in-place rewrite with an unchanged stat
+ * signature. The pre-query auto-refresh (`refresh.ts`) rebuilds and immediately
+ * queries the graph, so callers that rebuild must still invalidate explicitly.
  */
 export function invalidateGraphCaches(outDir: string): void {
   graphCache.delete(wiringPath(outDir));

test/graph-load-identity.test.ts

@@ -0,0 +1,95 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { mkdtempSync, mkdirSync, renameSync, rmSync, statSync, utimesSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { loadGraphCached, loadAskIndexCached, invalidateGraphCaches, __parseCount, __resetParseCounts } from "../src/graph/load.js";
+import { writeGraph, wiringPath, readGraph } from "../src/graph/write.js";
+import { writeAskIndex, askIndexPath, readAskIndex } from "../src/ask/index-file.js";
+import type { GraphV1 } from "../src/graph/types.js";
+
+const fixedTime = new Date("2023-11-14T22:13:20.000Z");
+function graph(id: string): GraphV1 {
+  return { version: 1, nodes: [{ id, name: id, kind: "function", path: `${id}.ts`, span: "L1-L1", signature: null, exported: true, origin: "ast", body_hash: id, summary_state: "pending", summary: null, crux: null }], edges: [] };
+}
+const readers = [
+  { kind: "graph", load: loadGraphCached, write: (dir: string, id: string) => writeGraph(graph(id), dir), path: wiringPath, id: (value: any) => value?.nodes[0]?.id, read: (dir: string) => readGraph(wiringPath(dir)), count: () => __parseCount.graph },
+  { kind: "ask", load: loadAskIndexCached, write: (dir: string, id: string) => writeAskIndex(dir, graph(id)), path: askIndexPath, id: (value: any) => value?.docs[0]?.id, read: readAskIndex, count: () => __parseCount.askIndex },
+];
+function fixture(t: any): string {
+  const dir = mkdtempSync(join(tmpdir(), "graft-cache-identity-"));
+  t.after(() => { invalidateGraphCaches(dir); rmSync(dir, { recursive: true, force: true }); });
+  return dir;
+}
+function stamp(path: string): void { utimesSync(path, fixedTime, fixedTime); }
+function identity(path: string) {
+  const s = statSync(path);
+  return { dev: s.dev, ino: s.ino, ctimeMs: s.ctimeMs, mtimeMs: s.mtimeMs, size: s.size };
+}
+
+for (const r of readers) {
+  test(`${r.kind}: same-size same-mtime atomic rename reloads and stays warm`, (t) => {
+    const dir = fixture(t), replacement = fixture(t);
+    r.write(dir, "alpha"); stamp(r.path(dir)); __resetParseCounts();
+    const first = r.load(dir);
+    assert.equal(r.id(first), "alpha"); assert.strictEqual(r.load(dir), first); assert.equal(r.count(), 1);
+    const before = identity(r.path(dir));
+    r.write(replacement, "bravo"); stamp(r.path(replacement));
+    renameSync(r.path(replacement), r.path(dir));
+    const after = identity(r.path(dir));
+    assert.equal(after.mtimeMs, before.mtimeMs); assert.equal(after.size, before.size);
+    assert.notEqual(after.ino, before.ino, "real replacement must have a distinct inode");
+    assert.equal(r.id(r.read(dir)), "bravo", "uncached production reader must see replacement bytes");
+    const second = r.load(dir);
+    assert.equal(r.id(second), "bravo", "cached loader must pick up atomic replacement");
+    assert.notStrictEqual(second, first); assert.equal(r.count(), 2);
+    assert.strictEqual(r.load(dir), second); assert.equal(r.count(), 2);
+  });
+  test(`${r.kind}: explicit invalidation forces an unchanged file to reparse`, (t) => {
+    const dir = fixture(t); r.write(dir, "alpha"); stamp(r.path(dir)); __resetParseCounts();
+    const first = r.load(dir); assert.equal(r.id(first), "alpha");
+    assert.strictEqual(r.load(dir), first); assert.equal(r.count(), 1);
+    invalidateGraphCaches(dir);
+    const second = r.load(dir); assert.deepEqual(second, first); assert.notStrictEqual(second, first); assert.equal(r.count(), 2);
+  });
+  test(`${r.kind}: missing files do not cache a miss or retain a removed value`, (t) => {
+    const dir = fixture(t); __resetParseCounts();
+    assert.equal(r.load(dir), null); assert.equal(r.load(dir), null); assert.equal(r.count(), 0);
+    r.write(dir, "alpha"); stamp(r.path(dir)); assert.equal(r.id(r.load(dir)), "alpha"); assert.equal(r.count(), 1);
+    rmSync(r.path(dir)); assert.equal(r.load(dir), null); assert.equal(r.count(), 1);
+    r.write(dir, "bravo"); stamp(r.path(dir)); assert.equal(r.id(r.load(dir)), "bravo"); assert.equal(r.count(), 2);
+  });
+  test(`${r.kind}: malformed JSON stays null and atomic repair is read`, (t) => {
+    const dir = fixture(t), repair = fixture(t); r.write(repair, "alpha");
+    const size = statSync(r.path(repair)).size;
+    mkdirSync(join(dir, r.kind === "graph" ? ".graph" : ".cache"), { recursive: true });
+    writeFileSync(r.path(dir), "x".repeat(size)); stamp(r.path(dir)); __resetParseCounts();
+    assert.equal(r.load(dir), null); assert.equal(r.load(dir), null); assert.equal(r.count(), 1);
+    stamp(r.path(repair)); renameSync(r.path(repair), r.path(dir));
+    assert.equal(r.id(r.load(dir)), "alpha"); assert.equal(r.count(), 2);
+  });
+}
+
+test("root paths isolate graph and ask values with identical size/mtime", (t) => {
+  const parent = fixture(t), left = join(parent, "left", "graft"), right = join(parent, "right", "graft");
+  t.after(() => { invalidateGraphCaches(left); invalidateGraphCaches(right); });
+  __resetParseCounts();
+  for (const r of readers) {
+    r.write(left, "alpha"); r.write(right, "bravo"); stamp(r.path(left)); stamp(r.path(right));
+    const a = identity(r.path(left)), b = identity(r.path(right));
+    assert.equal(a.size, b.size); assert.equal(a.mtimeMs, b.mtimeMs);
+    const one = r.load(left), two = r.load(right);
+    assert.equal(r.id(one), "alpha"); assert.equal(r.id(two), "bravo"); assert.notStrictEqual(one, two);
+    assert.strictEqual(r.load(left), one); assert.strictEqual(r.load(right), two); assert.equal(r.count(), 2);
+    invalidateGraphCaches(left); assert.equal(r.id(r.load(left)), "alpha");
+    assert.strictEqual(r.load(right), two); assert.equal(r.count(), 3);
+  }
+});
+
+test("real graph/ask rebuild plus explicit invalidation needs no sleep", (t) => {
+  const dir = fixture(t); __resetParseCounts();
+  for (const r of readers) { r.write(dir, "alpha"); stamp(r.path(dir)); assert.equal(r.id(r.load(dir)), "alpha"); }
+  for (const r of readers) { r.write(dir, "bravo"); stamp(r.path(dir)); }
+  invalidateGraphCaches(dir);
+  for (const r of readers) { const value = r.load(dir); assert.equal(r.id(value), "bravo"); assert.strictEqual(r.load(dir), value); assert.equal(r.count(), 2); }
+});