From 01bb163a301b6a4b9e83fa07eb458da2ed31927f Mon Sep 17 00:00:00 2001 From: insuffer Date: Thu, 12 Nov 2026 11:43:38 +0800 Subject: [PATCH] fix(graph): refuse to refresh a graph from a foreign root, and scream when a rebuild empties it (#438) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `graft ask` without its repo-root positional anchors the refresh on the cwd. When that root is unrelated to the graph's home, the drift probe reads "every file changed" and the rebuild writes an empty graph over a valid one — with the per-file cards surviving to mask the damage. The fingerprint now records the root its build ran on, and the refresh gate compares it against the root a query resolved, as real paths: macOS symlinks /tmp to /private/tmp, so a graph built through one name and queried through the other is the same graph, and comparing lexically would refuse to refresh it on every query from then on. A mismatch leaves the graph untouched and the note names the one argument that makes the refresh honest. A seeded worktree re-homes its copied sidecar, since the seed copies the parent checkout's fingerprint verbatim and its prints are the diff base the worktree wants. Behind that guard there is a safety net for the case it cannot catch: a legitimate-looking refresh that still empties the graph, because every source was deleted or an --only-dir whitelist no longer matches. The rebuild succeeded, so that is not an error — but "silently emptied" is the worst failure mode there is, because the cards survive and the graph looks intact until a query misses. The note says what the root now contains and what to run. Refs #438 --- src/graph/build.ts | 2 +- src/graph/fingerprint.ts | 89 +++++++++++++++++++++- src/graph/refresh.ts | 48 +++++++++++- test/graph-root-guard.test.ts | 138 ++++++++++++++++++++++++++++++++++ 4 files changed, 272 insertions(+), 5 deletions(-) create mode 100644 test/graph-root-guard.test.ts diff --git a/src/graph/build.ts b/src/graph/build.ts index ac007870c..83c6dc418 100644 --- a/src/graph/build.ts +++ b/src/graph/build.ts @@ -436,7 +436,7 @@ export async function buildGraph( // these source bytes." Nothing about the projections below — which is why it is // safe to write here, and why `graphOnly` builds (the query path, which stops // right after this line) are still recorded as fresh. - writeFingerprint(outDir, entries, opts.onlyDirs, extensions); + writeFingerprint(outDir, entries, opts.onlyDirs, extensions, root); // Tier-2 passive surface: project the nodes into per-file markdown cards, and // refresh the INDEX roster. Pure projection — no LLM, no network. diff --git a/src/graph/fingerprint.ts b/src/graph/fingerprint.ts index 012960800..00a106b91 100644 --- a/src/graph/fingerprint.ts +++ b/src/graph/fingerprint.ts @@ -18,7 +18,8 @@ * degrade safely: no fingerprint → "unknown, rebuild"; no parse cache → the * rebuild is just cold. */ -import { join } from "node:path"; +import { readdirSync, realpathSync } from "node:fs"; +import { join, resolve } from "node:path"; import { CACHE_DIR } from "../context/node-file.js"; import { contentHash } from "../util/id.js"; import { readSourceFile } from "../util/source.js"; @@ -27,7 +28,7 @@ import { extractorStamp, pruneSidecars, type ExtractEntry } from "./extract-cach import { listSourceStats } from "./source-files.js"; export const FINGERPRINT_PREFIX = "fingerprint"; -const FINGERPRINT_VERSION = 1; +const FINGERPRINT_VERSION = 2; /** The identity this graft's prints are filed under. Unlike the extract memo, a * missing extractor identity is *not* disqualifying here: freshness is a claim about @@ -56,6 +57,10 @@ export interface Fingerprint { onlyDirs?: string[]; /** Normalized source extension selection (`--extensions`); absent = all supported. */ extensions?: string[]; + /** Absolute source root this build ran on. The refresh gate compares it against + * the root a query resolved: a graph built for repo A must never be rebuilt from + * repo B's file set (#438). */ + root?: string; } /** What moved since the last build. Empty in all three arrays = nothing to do. */ @@ -91,6 +96,7 @@ export function writeFingerprint( entries: Record, onlyDirs?: string[], extensions?: string[], + root?: string, ): boolean { const files: Record = {}; for (const [rel, e] of Object.entries(entries)) files[rel] = [e.size, e.mtimeMs, e.hash]; @@ -98,6 +104,7 @@ export function writeFingerprint( const record: Fingerprint = { version: FINGERPRINT_VERSION, extractor: stamp(), files }; if (onlyDirs && onlyDirs.length > 0) record.onlyDirs = onlyDirs; if (extensions && extensions.length > 0) record.extensions = extensions; + if (root) record.root = root; writeJsonAtomic(fingerprintPath(outDir), record, true); pruneSidecars(join(outDir, CACHE_DIR), FINGERPRINT_PREFIX); return true; @@ -106,6 +113,84 @@ export function writeFingerprint( } } +/** + * The path as the filesystem answers it: symlinks resolved, so `/tmp/x` and + * `/private/tmp/x` compare equal on macOS, where they are the same directory. + * + * `realpathSync` throws for a path that does not exist, and the guard has to + * answer for a root the user may have mistyped, so fall back to `resolve` there + * — a lexical normalization still catches trailing slashes and `.` segments, + * which is the rest of what this comparison is for. + */ +function realPath(p: string): string { + try { + return realpathSync(p); + } catch { + return resolve(p); + } +} + +/** + * Is the graph still homed at this root? + * + * Compared as real paths: macOS symlinks `/tmp` to `/private/tmp`, so a graph + * built through one name and queried through the other is the same graph, and + * treating it as a move would refuse to refresh it forever. Case-insensitive on + * Windows, where the same directory legitimately answers to different casing. + */ +export function sameGraphRoot(recorded: string | undefined, resolved: string): boolean { + if (!recorded) return true; // a graph that never recorded a root is not evidence of a move + const a = realPath(recorded); + const b = realPath(resolved); + if (a === b) return true; + return process.platform === "win32" && a.toLowerCase() === b.toLowerCase(); +} + +/** + * The refresh gate's answer when the graph belongs to a different root (#438). + * + * Returns an actionable stderr note, or null when the roots match (or nothing was + * recorded). The caller must NOT rebuild on a mismatch: an `ask` that lost its + * repo-root positional anchors on the cwd concludes every file changed and + * rebuilds the graph empty in a second — with the per-file cards surviving to + * mask the damage. Refusing to touch the graph keeps it intact and tells the + * user the one argument that makes the refresh honest. + */ +export function graphRootGuardNote(recorded: string | undefined, resolved: string): string | null { + if (sameGraphRoot(recorded, resolved)) return null; + return ( + `graph was built for ${recorded}, not ${resolve(resolved)} — ` + + `answering from the existing graph; to refresh it, pass its root: ` + + `graft ask ${recorded}` + ); +} + +/** Re-home a seeded worktree's copied fingerprint sidecars on the worktree root. + * The seed copies the parent checkout's fingerprint verbatim (its prints are the + * diff base the worktree refresh wants), so without this the copied `root` would + * keep every worktree query looking like a foreign root. Best-effort: a failure + * costs the next query its root guard, never a wrong answer. */ +export function restampFingerprintRoot(outDir: string, root: string): void { + const cacheDir = join(outDir, CACHE_DIR); + let entries: string[]; + try { + entries = readdirSync(cacheDir).filter((name) => name.startsWith(`${FINGERPRINT_PREFIX}.`)); + } catch { + return; + } + for (const name of entries) { + const path = join(cacheDir, name); + try { + const record = readJson(path); + if (!record || typeof record !== "object" || record.root === root) continue; + record.root = root; + writeJsonAtomic(path, record, true); + } catch { + /* one sidecar failing to re-home must not fail the seed */ + } + } +} + /** `GRAFT_REFRESH=hash` — never trust a stat, confirm every file by its bytes. */ export function alwaysHash(): boolean { return process.env.GRAFT_REFRESH === "hash"; diff --git a/src/graph/refresh.ts b/src/graph/refresh.ts index 8dddf57ba..a49796bde 100644 --- a/src/graph/refresh.ts +++ b/src/graph/refresh.ts @@ -47,11 +47,19 @@ import { homeFolderRefusal } from "../init-guard.js"; import { acquireLockIn, lockHeldIn, releaseLockIn } from "../util/state.js"; import { CACHE_DIR } from "../context/node-file.js"; import { buildGraph } from "./build.js"; -import { driftCount, isClean, probeDrift, readFingerprint, type Drift } from "./fingerprint.js"; +import { + driftCount, + graphRootGuardNote, + isClean, + probeDrift, + readFingerprint, + restampFingerprintRoot, + type Drift, +} from "./fingerprint.js"; import { invalidateGraphCaches, loadGraphCached } from "./load.js"; import type { RefreshRunArgs } from "./refresh-run.js"; import { seedGraph, type SeedResult } from "./seed.js"; -import { wiringPath } from "./write.js"; +import { readGraph, wiringPath } from "./write.js"; import { cmd, tag } from "../brand.js"; /** How long to wait for another process's in-flight rebuild before giving up and @@ -292,6 +300,25 @@ export async function ensureFreshGraph(root: string, opts: RefreshOptions = {}): } } const seedNote = seededFrom ? `copied the graph from the main checkout (${seededFrom})` : builtNote; + if (seededFrom) { + // The seed copied the parent checkout's fingerprint verbatim — its prints + // are the diff base this worktree's refresh wants — so only the recorded + // home needs re-homing, or every worktree query would look foreign. + restampFingerprintRoot(outDir, dir); + } + + // Root guard (#438): never rebuild this graph from a different root's file + // set. An `ask` that lost its repo-root positional anchors on the cwd, when + // that root is unrelated to the graph's home, makes the drift probe read + // "every file changed" and the rebuild writes an empty graph over a valid + // one — with the per-file cards surviving to mask the damage. The graph + // stays untouched and the note names the root that would make the refresh + // honest. + const recordedRoot = readFingerprint(outDir)?.root; + const rootGuard = graphRootGuardNote(recordedRoot, dir); + if (rootGuard) { + return { refreshed: false, note: rootGuard }; + } // null = no fingerprint at all: a graph built before this mechanism existed, // or by a different extractor build. Rebuild once — that lays the fingerprint @@ -333,8 +360,25 @@ export async function ensureFreshGraph(root: string, opts: RefreshOptions = {}): // so a missing or obsolete extractor fingerprint cannot widen the build. const onlyDirs = readFingerprint(outDir)?.onlyDirs; const extensions = loadGraphCached(outDir)?.meta?.extensions; + const nodesBefore = readGraph(wiringPath(outDir))?.meta.nodeCount ?? 0; await rebuild({ dir, contextDir: opts.contextDir, onlyDirs, extensions }); invalidateGraphCaches(outDir); + // Safety net behind the root guard (#438): a legitimate-looking refresh can + // still empty a graph (every source deleted, or an `--only-dir` whitelist + // that no longer matches). The rebuild succeeded, so this is not an error — + // but "silently emptied" is the worst failure mode there is, because the + // per-file cards survive and the graph looks intact until a query misses. + const nodesAfter = readGraph(wiringPath(outDir))?.meta.nodeCount ?? 0; + if (nodesBefore > 0 && nodesAfter === 0) { + return { + refreshed: true, + drift: drift ?? undefined, + note: + `rebuilt the graph to 0 nodes (was ${nodesBefore}) — the sources this root ` + + `now contains do not match the graph; restore them or run 'graft build' from ` + + `the root the graph was built for`, + }; + } return { refreshed: true, drift: drift ?? undefined, note: seedNote }; } finally { unhook(); diff --git a/test/graph-root-guard.test.ts b/test/graph-root-guard.test.ts new file mode 100644 index 000000000..c17f6be14 --- /dev/null +++ b/test/graph-root-guard.test.ts @@ -0,0 +1,138 @@ +/** + * Root guard for the pre-query freshness check (#438). + * + * `graft ask` without its repo-root positional anchors the refresh on the cwd. + * When that root is unrelated to the graph's home, the drift probe reads "every + * file changed" and the rebuild writes an empty graph over a valid one — with + * the per-file cards surviving to mask the damage. These tests pin the pure + * decision layer: the fingerprint records the build's root, the guard refuses to + * rebuild across roots, and a seeded worktree re-homes its copied fingerprint. + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; + +import { + fingerprintPath, + graphRootGuardNote, + readFingerprint, + restampFingerprintRoot, + sameGraphRoot, + writeFingerprint, +} from "../src/graph/fingerprint.js"; + +test("a graph that never recorded a root is not treated as foreign", () => { + assert.equal(sameGraphRoot(undefined, "/repo"), true); + assert.equal(graphRootGuardNote(undefined, "/repo"), null); +}); + +test("the same root matches through trailing slashes and dot segments", () => { + assert.equal(sameGraphRoot("/repo", "/repo"), true); + assert.equal(sameGraphRoot("/repo", "/repo/"), true); + assert.equal(sameGraphRoot("/repo", "/repo/./"), true); + assert.equal(sameGraphRoot("/repo", "/repo/sub/.."), true); +}); + +/** The note shows the resolved cwd, which on Windows carries a drive letter + * and backslashes — the actionable form is the platform-native one. */ +const win = process.platform === "win32"; + +test("a different root is a mismatch and the note names the root to pass", () => { + assert.equal(sameGraphRoot("/repo-a", "/repo-b"), false); + const note = graphRootGuardNote("/repo-a", "/repo-b"); + assert.ok(note, "a mismatch must produce a note"); + assert.match(note, /graph was built for \/repo-a/); + assert.match(note, win ? /not [A-Za-z]:\\repo-b/ : /not \/repo-b/); + // actionable: the note contains the corrected invocation + assert.match(note, /graft ask \/repo-a/); +}); + +test("a mismatch note is normalized through resolve, not raw string equality", () => { + const note = graphRootGuardNote("/repo-a", "/repo-a/../repo-b"); + assert.ok(note); + assert.match(note, win ? /not [A-Za-z]:\\repo-b/ : /not .*\/repo-b/); +}); + +test("writeFingerprint records the root and readFingerprint returns it", () => { + const dir = mkdtempSync(join(tmpdir(), "graft-fp-root-")); + try { + const outDir = join(dir, "graft"); + writeFingerprint(outDir, {}, undefined, undefined, resolve(dir)); + const fp = readFingerprint(outDir); + assert.ok(fp, "the sidecar should be readable"); + assert.equal(fp!.root, resolve(dir)); + assert.equal(sameGraphRoot(fp!.root, dir), true); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("a pre-#438 fingerprint without a root reads as current-format-null via version", () => { + const dir = mkdtempSync(join(tmpdir(), "graft-fp-old-")); + try { + const outDir = join(dir, "graft"); + // Simulate the v1 sidecar this change invalidates: no `root` field. + writeFingerprint(outDir, {}, undefined, undefined, resolve(dir)); + const path = fingerprintPath(outDir); + const record = JSON.parse(readFileSync(path, "utf8")); + record.version = 1; + delete record.root; + writeFileSync(path, JSON.stringify(record)); + assert.equal(readFingerprint(outDir), null, "a v1 sidecar must force one honest rebuild"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("restampFingerprintRoot re-homes a seeded worktree's copied sidecar", () => { + const dir = mkdtempSync(join(tmpdir(), "graft-fp-restamp-")); + try { + const outDir = join(dir, "graft"); + writeFingerprint(outDir, {}, undefined, undefined, resolve("/main/checkout")); + assert.equal(readFingerprint(outDir)!.root, resolve("/main/checkout")); + + restampFingerprintRoot(outDir, resolve("/worktree")); + const fp = readFingerprint(outDir)!; + assert.equal(fp.root, resolve("/worktree")); + assert.equal(sameGraphRoot(fp.root, "/worktree"), true, "the worktree now owns its guard"); + assert.equal(sameGraphRoot(fp.root, "/main/checkout"), false, "and the parent no longer matches"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("restampFingerprintRoot is a no-op on a missing cache dir", () => { + const dir = mkdtempSync(join(tmpdir(), "graft-fp-nocache-")); + try { + assert.doesNotThrow(() => restampFingerprintRoot(join(dir, "graft"), "/repo")); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +/** A second name for the same directory must not read as a move. + * + * macOS symlinks `/tmp` to `/private/tmp`, so a graph built through one name and + * queried through the other is the same graph. Comparing lexically would refuse + * to refresh it on every query from then on, which is the failure this guards + * against. Skipped where the platform refuses to create the link at all (Windows + * without developer mode), since the decision is otherwise covered above. */ +test("a symlinked name for the same directory is not a foreign root", () => { + const dir = mkdtempSync(join(tmpdir(), "graft-fp-link-")); + const link = join(dir, "link"); + try { + try { + // "junction" needs no elevation on Windows; the default "dir" type does. + symlinkSync(dir, link, win ? "junction" : "dir"); + } catch (err) { + if ((err as NodeJS.ErrnoException).code !== "EPERM") throw err; + return; + } + assert.equal(sameGraphRoot(dir, link), true, "one directory, two names"); + assert.equal(graphRootGuardNote(dir, link), null, "so the refresh must not be refused"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +});