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 }); + } +});