diff --git a/.mex/ROUTER.md b/.mex/ROUTER.md index 6bd5d807..baf35a76 100644 --- a/.mex/ROUTER.md +++ b/.mex/ROUTER.md @@ -16,7 +16,7 @@ edges: condition: when starting a task — check the pattern index for a matching pattern file - target: patterns/release-readme-visuals.md condition: when refreshing the release README, badges, community links, or architecture illustrations -last_updated: 2026-09-06 +last_updated: 2026-09-09 --- # Session Bootstrap @@ -75,6 +75,21 @@ Then read this file fully before doing anything else in this session. the last trustworthy index behind one cross-process maintenance lease. - Targeted graph get/query/impact consumers use one provenance-bound immutable snapshot and discard output if graph or exact source identity changes. +- Graph reads separate engine identity from bounded, reportable shortfalls. A + store built by incompatible code still refuses every read. A store whose + config inputs drifted, whose files parsed partially, or whose indexed source + changed is answered and labelled: resolved edges are marked stale under config + drift, an incomplete parse reports its affected files, and drifted source is + excluded by an exhaustive path set the response names. Definitions, + containment and verified source stay unlabelled. Scope classifies through the + same predicate and refuses through the same record while keeping its own + per-file text-only fallback. +- Publication applies the same judgement: a candidate whose only fault is a + skipped or partially parsed file is published rather than discarded, and a + failed maintenance run reports the diagnostics that blocked it. +- Config inputs are identified by the fields that affect extraction rather than + by raw bytes, so a dependency bump or a reformat no longer invalidates an + index; anything unparseable falls back to exact bytes. - The graph half of Checkpoint 2 is working in the Project Hub: grouped symbol and source Search, the read-only Code workspace, structured graph Health, and explicit refresh/rebuild jobs all use the repository-bound GraphPort adapter. diff --git a/.mex/patterns/safe-graph-snapshot-evolution.md b/.mex/patterns/safe-graph-snapshot-evolution.md index 0ad064c8..9b0ddfd6 100644 --- a/.mex/patterns/safe-graph-snapshot-evolution.md +++ b/.mex/patterns/safe-graph-snapshot-evolution.md @@ -12,7 +12,7 @@ edges: condition: "when changing the graph data plane or its consumers" - target: "context/conventions.md" condition: "when verifying a graph implementation change" -last_updated: 2026-09-07 +last_updated: 2026-09-09 mex: id: mx_01M1M0CJP81C590FCKTSN5HA3Q type: pattern @@ -128,6 +128,58 @@ only to explicit maintenance workflows. enter the corpus policy hash, or changing it leaves a stale index silently describing files that are no longer in the corpus. Hash to the existing constant when nothing is configured, so existing indexes stay valid. +- A freshness input is not one kind of thing. Engine identity — schema, + compiler, extractor, resolver, grammar, corpus policy — says the store was + written by code that is gone, and must fail closed. Config content says the + build inputs moved under a store that still describes its source exactly. + Folding both into one hash means the second is served the punishment of the + first, and a dependency bump takes every structural read with it. +- Prove identity by reconstruction, not by a new stored field. Re-folding the + current inputs with a store's recorded config hash classifies stores written + before the check existed — which are exactly the stores that need it — and + covers inputs no snapshot records at all. +- Separate the race check from the freshness check inside one validation. Two + observations disagreeing with each other is a race; either of them disagreeing + with the stored snapshot is the question the caller already answered. Mixing + them reports a race that did not happen and refuses a read that was safe. +- A degraded answer must say which half of itself is degraded. Definitions, + containment and verified source bytes survive a config change; anything + reached by following an edge does not. Labelling everything is honest but + wastes a trustworthy answer; labelling nothing is a lie. +- Commit output under the class it was labelled with. If the store changes class + between opening and output, discard the response rather than relabelling it — + the records were built under a claim they no longer earn. +- Do not unify two gates by giving both the stricter one. Scope tolerates + drifted source because it re-admits a moved file as text-only evidence; the + targeted commands cannot, because they return exact node coordinates. One + vocabulary and one classifier is the unification; one tolerance is a + regression wearing its clothes. +- `degraded` is not `unusable`, and conflating them costs a repository its + graph twice over. A candidate whose only fault is a file the policy skipped + must publish, or the skip path produces a candidate the publish gate throws + away; a store with a partial parse must read, or one unparseable file answers + nothing. Enumerate which shortfalls are known, bounded and reportable, and + admit exactly those. +- Adding a diagnostic code is half the change. Every allowlist that enumerates + codes — publication, repair, refusal ranking — has to learn it in the same + commit, or the new code silently means "refuse". +- Serving around a gap requires the gap's *complete* extent. Excluding drifted + files is only safe while the drifted list is exhaustive, so bind it to the + ceiling that truncates the list and refuse past it. A partial exclusion set is + worse than refusing outright. +- Distinguish out-of-date from incomplete when labelling. Drifted config makes + resolved edges untrustworthy; an unfinished parse makes the answer smaller + while everything in it stays true. One label for both teaches the reader to + ignore the label. +- Hash a config input by what it changes, not by its bytes — and fail towards + over-invalidation. A version bump or a reindent invalidating an index is + noise; a resolution-affecting field missing from the projection is a stale + index reading as current with no label at all. +- A re-resolved path comparison is a name check, not an identity check. On a + case-insensitive volume the same file can come back spelled differently — a + path routed through the TypeScript compiler host arrives lowercased — and a + byte comparison then rejects a file whose device, inode, size and timestamps + all match. - Wall-clock status timings vary by machine and process-start overhead. Keep the benchmark non-gating, record its environment, and protect correctness with deterministic race, non-mutation, and bounded-work tests. diff --git a/docs/design/graph-freshness-recovery.md b/docs/design/graph-freshness-recovery.md index ba096130..991458cb 100644 --- a/docs/design/graph-freshness-recovery.md +++ b/docs/design/graph-freshness-recovery.md @@ -26,8 +26,8 @@ database is retained locally as `.mex/graph.db.recovery-*`. ## Targeted retrieval handshake -`graph get`, `graph query`, and `impact` first require a stable `fresh` status -observation. They then adopt one immutable SQLite connection for graph and +`graph get`, `graph query`, and `impact` first require a stable status +observation that is either `fresh` or config-drifted (below). They then adopt one immutable SQLite connection for graph and grounding reads, bind it to the inspected inode and exact `graph_snapshot_v1` bytes, and buffer the complete JSONL response. Source ranges come from one contained, fd-stable byte buffer whose UTF-8 decoded hash matches the indexed @@ -36,9 +36,66 @@ validation; any mismatch discards the whole response and emits one bounded `GRAPH_UNAVAILABLE` record. `graph scope` deliberately retains its existing stale-file text-only fallback. -It now uses a single stable immutable database snapshot, but it does not claim -that stale live text is an indexed graph fact. Retrieval ranking and successful -protocol-v3 records remain unchanged. +It uses a single stable immutable database snapshot, but it does not claim +that stale live text is an indexed graph fact. It is not bound to the exact +freshness handshake: that fallback is what lets it answer while source files +are being edited, and binding it would turn one edited file into a refused +retrieval. It classifies build identity through the same predicate as the +targeted commands and refuses through the same record. Retrieval ranking and +successful protocol-v3 records remain unchanged. + +## Reading a store that is not provably fresh + +The build manifest folds seven inputs. Six are engine identity — schema, +compiler, extractor and resolver versions, grammar, and corpus policy — and a +difference in any of them means the store was written by code that is no longer +here. The seventh is the content of every `package.json`, `tsconfig*.json` and +`jsconfig*.json` in the repository, which moves for reasons that change nothing +about the graph: a dependency version, a script, a reformat. + +Collapsing all seven into one comparison made a dependency bump indistinguishable +from an incompatible store, and both refused every structural read until a full +rebuild. A store is now classified as config-drifted when it would read `fresh` +except that its config inputs moved: engine identity reproduces from the current +inputs and that store's recorded config hash, the indexed corpus, branch, corpus +digest and grammar all still match, parse health is clean, and every inspection +completed. Anything short of that still refuses. + +Two further shortfalls are bounded in the same way. A store whose files parsed +partially is *incomplete* rather than out of date — every fact in it is still +true — so it is read and the response reports how many files are affected and +which failed. A store whose indexed source has changed is read by excluding the +complete set of drifted paths and answering from the rest; the response names +every file it left out, a node whose own file drifted is reported as excluded +rather than missing, and a target that resolves only into excluded files says +so. Completeness of that set is the safety property, so it is bound to the +change-path ceiling: a truncated change list cannot be exhaustively excluded +from and refuses as before. + +A degraded store is bound and read exactly like a fresh one, and the response +says so. Definitions, containment and returned source are unlabelled: +they do not depend on compiler configuration, and the source bytes are already +proven identical to what was indexed. Resolution does depend on it — `paths`, +`moduleResolution`, `references` and a package `type` decide what a reference +binds to — so callers, call relations, flows and unresolved references carry +`stale: true`, and the response opens with a `status` record naming the drift +and the recovery command. Every one of those fields is absent while the graph is +fresh. + +Publication applies the same judgement in the other direction. A candidate whose +only fault is a file the corpus policy skipped, or one that parsed partially, is +published: refusing would discard every other file's facts to punish a gap a +rebuild would reproduce exactly. Corpus-wide breaches and incomplete inspections +still block, because those mean the observation itself is untrustworthy. + +Config inputs are identified by the fields that decide what the compiler +resolves, not by their bytes, so a dependency version, a script or a reindent +does not invalidate an index. Anything unparseable or unrecognized falls back to +exact bytes: over-invalidation is noisy, but under-invalidation would serve a +stale index as current with nothing to say otherwise. + +Reading a drifted store writes nothing to it. The label is not a substitute for +`mex graph refresh`; it is what the graph can honestly say until then. ## Evaluator identity diff --git a/src/__tests__/paths.test.ts b/src/__tests__/paths.test.ts new file mode 100644 index 00000000..f7e6f3ea --- /dev/null +++ b/src/__tests__/paths.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, it } from "vitest"; +import { isSameResolvedPath, toPosix } from "../paths.js"; + +const caseInsensitive = process.platform === "win32" || process.platform === "darwin"; + +describe("isSameResolvedPath", () => { + it("accepts identical paths", () => { + expect(isSameResolvedPath("/a/b/c.ts", "/a/b/c.ts")).toBe(true); + }); + + it("rejects different paths", () => { + expect(isSameResolvedPath("/a/b/c.ts", "/a/b/d.ts")).toBe(false); + }); + + it("treats a case-only difference as the same name only where the volume does", () => { + // A path that reached the graph through the TypeScript compiler host + // arrives lowercased; on a case-insensitive volume it names the same file, + // and comparing it as bytes once failed an entire repository's build. + expect(isSameResolvedPath("C:\\Users\\a\\File.ts", "c:\\users\\a\\file.ts")) + .toBe(caseInsensitive); + }); + + it("handles a missing side without throwing", () => { + expect(isSameResolvedPath(null, null)).toBe(true); + expect(isSameResolvedPath(null, "/a")).toBe(false); + expect(isSameResolvedPath("/a", null)).toBe(false); + }); +}); + +describe("toPosix", () => { + it("leaves forward-slash paths alone", () => { + expect(toPosix("src/graph/status.ts")).toBe("src/graph/status.ts"); + }); +}); diff --git a/src/cli.ts b/src/cli.ts index 4063ddd3..568e7290 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -495,7 +495,8 @@ const graphCommand = program const { runGraph } = await import("./graph/cli-graph.js"); await runGraph({ root: opts.root, json: opts.json }); } catch (err) { - console.error((err as Error).message); + const { describeGraphMaintenanceFailure } = await import("./graph/cli-graph.js"); + console.error(describeGraphMaintenanceFailure(err)); process.exit(1); } }); @@ -531,7 +532,8 @@ graphCommand json: opts.json ?? graphCommand.opts().json, }); } catch (err) { - console.error((err as Error).message); + const { describeGraphMaintenanceFailure } = await import("./graph/cli-graph.js"); + console.error(describeGraphMaintenanceFailure(err)); process.exit(1); } }); @@ -549,7 +551,8 @@ graphCommand json: opts.json ?? graphCommand.opts().json, }); } catch (err) { - console.error((err as Error).message); + const { describeGraphMaintenanceFailure } = await import("./graph/cli-graph.js"); + console.error(describeGraphMaintenanceFailure(err)); process.exit(1); } }); diff --git a/src/graph/__tests__/config-drift-observation.test.ts b/src/graph/__tests__/config-drift-observation.test.ts new file mode 100644 index 00000000..5da1d37e --- /dev/null +++ b/src/graph/__tests__/config-drift-observation.test.ts @@ -0,0 +1,170 @@ +import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { openSqlite } from "../db/sqlite.js"; +import { createGraphEngine } from "../engine-impl.js"; +import { + GRAPH_SNAPSHOT_METADATA_KEY, + parseGraphSnapshot, + serializeGraphSnapshot, + type GraphSnapshot, +} from "../snapshot.js"; +import { inspectGraphStatusWithFreshObservation } from "../status.js"; + +const NOW = new Date("2026-09-08T12:00:00.000Z"); +const roots: string[] = []; + +afterEach(() => { + for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }); +}); + +function write(root: string, path: string, content: string): void { + const absolutePath = join(root, path); + mkdirSync(dirname(absolutePath), { recursive: true }); + writeFileSync(absolutePath, content); +} + +async function project(): Promise { + const root = mkdtempSync(join(tmpdir(), "mex-graph-config-drift-")); + roots.push(root); + write(root, "package.json", JSON.stringify({ name: "fixture", dependencies: { dep: "1.0.0" } })); + write(root, "tsconfig.json", JSON.stringify({ compilerOptions: { strict: true } })); + write(root, "src/a.ts", "export function alpha(): number {\n return beta();\n}\n" + + "export function beta(): number {\n return 1;\n}\n"); + const engine = createGraphEngine({ rootDir: root }); + try { + await engine.build(); + } finally { + engine.close(); + } + return root; +} + +async function inspect(root: string) { + return inspectGraphStatusWithFreshObservation({ projectRoot: root, now: NOW }); +} + +/** + * Change a config field that genuinely affects extraction. + * + * A dependency *version* deliberately no longer registers: config inputs are + * identified by the fields that decide what the compiler resolves. `type` is + * one of those, so this is drift the graph must notice. + */ +function driftConfig(root: string): void { + write(root, "package.json", JSON.stringify({ + name: "fixture", type: "commonjs", dependencies: { dep: "1.0.0" }, + })); +} + +function updateSnapshot(root: string, update: (snapshot: GraphSnapshot) => GraphSnapshot): void { + const db = openSqlite(join(root, ".mex", "graph.db")); + try { + const row = db.prepare("SELECT value FROM project_metadata WHERE key = ?") + .get(GRAPH_SNAPSHOT_METADATA_KEY) as { value: string }; + const snapshot = parseGraphSnapshot(row.value); + if (!snapshot) throw new Error("test fixture has no valid graph snapshot"); + db.prepare("UPDATE project_metadata SET value = ?, updated_at = ? WHERE key = ?") + .run(serializeGraphSnapshot(update(snapshot)), NOW.getTime(), GRAPH_SNAPSHOT_METADATA_KEY); + } finally { + db.close(); + } +} + +describe("config-drift read observation", () => { + it("binds a fresh store as fresh and never as drifted", async () => { + const root = await project(); + const inspection = await inspect(root); + expect(inspection.graphStatus.status).toBe("fresh"); + expect(inspection.freshObservation).not.toBeNull(); + expect(inspection.degradedObservation ?? null).toBeNull(); + }); + + it("binds a store whose only drift is config content", async () => { + const root = await project(); + driftConfig(root); + const inspection = await inspect(root); + expect(inspection.graphStatus.status).toBe("stale"); + expect(inspection.graphStatus.changes).toMatchObject({ + total: 0, + configChanged: true, + manifestChanged: true, + grammarChanged: false, + branchChanged: false, + }); + expect(inspection.freshObservation).toBeNull(); + const observed = inspection.degradedObservation; + expect(observed).not.toBeNull(); + expect(observed!.degradations).toEqual(["config-drift"]); + expect(observed!.token.snapshotHash).toMatch(/^[0-9a-f]{64}$/); + expect(parseGraphSnapshot(observed!.token.snapshotRaw)).not.toBeNull(); + }); + + it("is deterministic across repeated inspections of one drifted store", async () => { + const root = await project(); + driftConfig(root); + const first = await inspect(root); + const second = await inspect(root); + expect(second.degradedObservation).toEqual(first.degradedObservation); + }); + + it("refuses to bind when engine identity cannot be reproduced", async () => { + const root = await project(); + driftConfig(root); + updateSnapshot(root, (snapshot) => ({ ...snapshot, manifestHash: "0".repeat(64) })); + const inspection = await inspect(root); + expect(inspection.graphStatus.status).toBe("stale"); + expect(inspection.graphStatus.changes.configChanged).toBe(true); + expect(inspection.freshObservation).toBeNull(); + expect(inspection.degradedObservation ?? null).toBeNull(); + }); + + it("refuses to bind when the grammar also moved", async () => { + const root = await project(); + driftConfig(root); + updateSnapshot(root, (snapshot) => ({ ...snapshot, grammarHash: "0".repeat(64) })); + const inspection = await inspect(root); + expect(inspection.graphStatus.changes.grammarChanged).toBe(true); + expect(inspection.degradedObservation ?? null).toBeNull(); + }); + + it("binds a store with drifted source, and reports the exact drifted paths", async () => { + const root = await project(); + driftConfig(root); + write(root, "src/a.ts", "export function alpha(): number {\n return 2;\n}\n"); + const inspection = await inspect(root); + expect(inspection.graphStatus.status).toBe("stale"); + expect(inspection.graphStatus.changes.total).toBeGreaterThan(0); + const observed = inspection.degradedObservation; + expect(observed).not.toBeNull(); + expect(observed!.degradations).toEqual(["config-drift", "source-drift"]); + // Complete, because a reader answers by excluding exactly this set. + expect(observed!.driftedSources).toEqual(["src/a.ts"]); + }); + + it("counts a new unindexed file as drift and names it", async () => { + const root = await project(); + driftConfig(root); + write(root, "src/b.ts", "export const b = 1;\n"); + const inspection = await inspect(root); + const observed = inspection.degradedObservation; + expect(observed).not.toBeNull(); + expect(observed!.degradations).toContain("source-drift"); + expect(observed!.driftedSources).toEqual(["src/b.ts"]); + }); + + it("refuses to bind when more paths changed than the change list can carry", async () => { + const root = await project(); + for (let index = 0; index < 6; index += 1) { + write(root, `src/extra-${index}.ts`, `export const extra${index} = ${index};\n`); + } + // A truncated change list cannot be excluded from exhaustively, so the + // store stops being serveable rather than being served incompletely. + const inspection = await inspectGraphStatusWithFreshObservation({ + projectRoot: root, now: NOW, maxChangedPaths: 3, + }); + expect(inspection.graphStatus.changes.truncated).toBe(true); + expect(inspection.degradedObservation ?? null).toBeNull(); + }); +}); diff --git a/src/graph/__tests__/config-identity.test.ts b/src/graph/__tests__/config-identity.test.ts new file mode 100644 index 00000000..ace5f447 --- /dev/null +++ b/src/graph/__tests__/config-identity.test.ts @@ -0,0 +1,192 @@ +import { describe, expect, it } from "vitest"; +import { graphConfigIdentity, graphConfigKind } from "../config-identity.js"; + +const identity = (path: string, value: unknown): string => + graphConfigIdentity(path, typeof value === "string" ? value : JSON.stringify(value, null, 2)); + +/** Same file, one field changed: does the graph consider it a different build? */ +const changes = (path: string, before: unknown, after: unknown): boolean => + identity(path, before) !== identity(path, after); + +describe("graph config identity", () => { + it("classifies the three config shapes the corpus policy admits", () => { + expect(graphConfigKind("package.json")).toBe("package"); + expect(graphConfigKind("packages/api/package.json")).toBe("package"); + expect(graphConfigKind("tsconfig.json")).toBe("tsconfig"); + expect(graphConfigKind("tsconfig.build.json")).toBe("tsconfig"); + expect(graphConfigKind("jsconfig.json")).toBe("tsconfig"); + expect(graphConfigKind("packages/api/tsconfig.base.json")).toBe("tsconfig"); + expect(graphConfigKind("some-other.json")).toBe("unknown"); + }); + + describe("a change that can move an edge still invalidates", () => { + const base = { + extends: "./tsconfig.base.json", + include: ["src/**/*.ts"], + exclude: ["dist"], + files: ["src/index.ts"], + references: [{ path: "../api" }], + compilerOptions: { + paths: { "@app/*": ["src/*"] }, + baseUrl: ".", + moduleResolution: "NodeNext", + module: "NodeNext", + target: "ES2022", + jsx: "react-jsx", + allowJs: true, + checkJs: false, + }, + }; + const withCompilerOption = (option: string, value: unknown): unknown => + ({ ...base, compilerOptions: { ...base.compilerOptions, [option]: value } }); + + // One case per field, so a field dropped from the projection fails here + // rather than silently serving a stale index as current. + const cases: Array<[string, unknown]> = [ + ["extends", { ...base, extends: "./tsconfig.other.json" }], + ["include", { ...base, include: ["lib/**/*.ts"] }], + ["exclude", { ...base, exclude: ["build"] }], + ["files", { ...base, files: ["src/main.ts"] }], + ["references", { ...base, references: [{ path: "../web" }] }], + ["paths", withCompilerOption("paths", { "@app/*": ["lib/*"] })], + ["baseUrl", withCompilerOption("baseUrl", "./src")], + ["moduleResolution", withCompilerOption("moduleResolution", "Bundler")], + ["module", withCompilerOption("module", "ESNext")], + ["target", withCompilerOption("target", "ES2020")], + ["jsx", withCompilerOption("jsx", "preserve")], + ["allowJs", withCompilerOption("allowJs", false)], + ["checkJs", withCompilerOption("checkJs", true)], + ]; + for (const [field, after] of cases) { + it(field, () => { + expect(changes("tsconfig.json", base, after)).toBe(true); + }); + } + + it("removing a significant field", () => { + const { compilerOptions, ...withoutCompilerOptions } = base; + expect(changes("tsconfig.json", base, withoutCompilerOptions)).toBe(true); + expect(compilerOptions.baseUrl).toBe("."); + }); + + it("reordering an array whose order is meaningful", () => { + const before = { compilerOptions: { paths: { "@app/*": ["src/*", "lib/*"] } } }; + const after = { compilerOptions: { paths: { "@app/*": ["lib/*", "src/*"] } } }; + expect(changes("tsconfig.json", before, after)).toBe(true); + }); + }); + + describe("a package.json change that can move an edge still invalidates", () => { + const base = { + name: "pkg", + version: "1.0.0", + type: "module", + workspaces: ["packages/*"], + imports: { "#internal": "./src/internal.js" }, + exports: { ".": "./src/index.js" }, + dependencies: { alpha: "^1.0.0" }, + devDependencies: { beta: "^2.0.0" }, + }; + const cases: Array<[string, unknown]> = [ + ["type", { ...base, type: "commonjs" }], + ["workspaces", { ...base, workspaces: ["apps/*"] }], + ["imports", { ...base, imports: { "#internal": "./lib/internal.js" } }], + ["exports", { ...base, exports: { ".": "./lib/index.js" } }], + ["a new dependency name", { ...base, dependencies: { alpha: "^1.0.0", gamma: "^1.0.0" } }], + ["a removed dependency name", { ...base, dependencies: {} }], + ]; + for (const [field, after] of cases) { + it(field, () => { + expect(changes("package.json", base, after)).toBe(true); + }); + } + }); + + describe("a change that cannot move an edge does not invalidate", () => { + const pkg = { + name: "pkg", + version: "1.0.0", + type: "module", + dependencies: { alpha: "^1.0.0" }, + scripts: { build: "tsup" }, + }; + + it("a dependency moving between dependency maps", () => { + // Which map declares a name does not change what a specifier resolves + // to; only whether the name is declared at all does. + expect(changes("package.json", + { ...pkg, dependencies: { alpha: "^1.0.0" } }, + { ...pkg, dependencies: {}, peerDependencies: { alpha: "^1.0.0" } })).toBe(false); + }); + + it("a dependency version bump", () => { + expect(changes("package.json", pkg, { ...pkg, dependencies: { alpha: "^1.2.3" } })).toBe(false); + }); + + it("the package's own version", () => { + expect(changes("package.json", pkg, { ...pkg, version: "2.0.0" })).toBe(false); + }); + + it("scripts, description, author and license", () => { + expect(changes("package.json", pkg, { + ...pkg, + scripts: { build: "tsup --watch", test: "vitest" }, + description: "a description", + author: "someone", + license: "MIT", + })).toBe(false); + }); + + it("key order and indentation", () => { + const reordered = JSON.stringify({ + dependencies: { alpha: "^1.0.0" }, type: "module", scripts: { build: "tsup" }, + version: "1.0.0", name: "pkg", + }); + expect(graphConfigIdentity("package.json", JSON.stringify(pkg, null, 4))) + .toBe(graphConfigIdentity("package.json", reordered)); + }); + + it("tsconfig comments, trailing commas and insignificant options", () => { + const withComments = `{ + // the compiler options this project uses + "compilerOptions": { + "moduleResolution": "NodeNext", + "strict": true, + "noEmit": true, + }, +}`; + const plain = JSON.stringify({ + compilerOptions: { moduleResolution: "NodeNext", strict: false, declaration: true }, + }); + expect(graphConfigIdentity("tsconfig.json", withComments)) + .toBe(graphConfigIdentity("tsconfig.json", plain)); + }); + }); + + describe("anything it cannot understand falls back to exact bytes", () => { + it("an unparseable config", () => { + const broken = "{ not json at all"; + expect(graphConfigIdentity("tsconfig.json", broken)).toBe(broken); + expect(changes("tsconfig.json", broken, "{ also not json")).toBe(true); + }); + + it("a config that is not an object", () => { + expect(graphConfigIdentity("package.json", "[1, 2, 3]")).toBe("[1, 2, 3]"); + }); + + it("a malformed compilerOptions", () => { + const malformed = JSON.stringify({ compilerOptions: "nonsense" }); + expect(graphConfigIdentity("tsconfig.json", malformed)).toBe(malformed); + }); + + it("a malformed dependency map", () => { + const malformed = JSON.stringify({ dependencies: ["alpha"] }); + expect(graphConfigIdentity("package.json", malformed)).toBe(malformed); + }); + + it("a file name the policy does not recognize", () => { + const source = JSON.stringify({ anything: true }); + expect(graphConfigIdentity("other.json", source)).toBe(source); + }); + }); +}); diff --git a/src/graph/__tests__/graph-v2-integrity.test.ts b/src/graph/__tests__/graph-v2-integrity.test.ts index c11e79c0..4062049d 100644 --- a/src/graph/__tests__/graph-v2-integrity.test.ts +++ b/src/graph/__tests__/graph-v2-integrity.test.ts @@ -565,7 +565,7 @@ describe("graph construction integration", () => { db.close(); }); - it("forces a deterministic rebuild when compiler configuration changes without a source edit", async () => { + it("labels retrieval, and still re-stages, when compiler configuration changes without a source edit", async () => { const root = temporaryRoot("mex-graph-manifest-"); mkdirSync(join(root, "src"), { recursive: true }); writeFileSync(join(root, "package.json"), JSON.stringify({ name: "manifest-fixture", type: "module" })); @@ -593,11 +593,19 @@ describe("graph construction integration", () => { compilerOptions: { target: "ES2022", module: "NodeNext", moduleResolution: "NodeNext" }, include: ["src/**/*.ts"], })); + // `moduleResolution` decides what a cross-file reference binds to, so this + // is a real change to the meaning of resolved edges. Retrieval says so and + // keeps answering from a store whose indexed source is still exact. const staleOutput: string[] = []; runGraphScope("manifestNeedle", root, { write: (line) => staleOutput.push(line) }); - expect(staleOutput.map((line) => JSON.parse(line))).toEqual([ - expect.objectContaining({ code: "GRAPH_REBUILD_REQUIRED", recoveryCommand: "mex graph" }), - ]); + const staleRecords = staleOutput.map((line) => JSON.parse(line) as Record); + expect(staleRecords.some((record) => record.type === "error")).toBe(false); + expect(staleRecords).toContainEqual(expect.objectContaining({ + type: "status", + graphStatus: "stale", + reasons: ["config-drift"], + recoveryCommand: "mex graph refresh", + })); const refreshEngine = createGraphEngine({ rootDir: root }); expect(await refreshEngine.sync(["tsconfig.json"])).toMatchObject({ filesIndexed: 1 }); refreshEngine.close(); diff --git a/src/graph/__tests__/manifest-identity.test.ts b/src/graph/__tests__/manifest-identity.test.ts new file mode 100644 index 00000000..d880c9f3 --- /dev/null +++ b/src/graph/__tests__/manifest-identity.test.ts @@ -0,0 +1,84 @@ +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { + graphManifest, + graphManifestDiffersOnlyByConfig, + graphManifestHash, + type GraphManifest, +} from "../engine-impl.js"; + +describe("graph manifest identity", () => { + let root: string; + + beforeAll(() => { + root = mkdtempSync(join(tmpdir(), "mex-manifest-identity-")); + writeFileSync(join(root, "package.json"), JSON.stringify({ name: "fixture", version: "1.0.0" })); + }); + + afterAll(() => { + rmSync(root, { recursive: true, force: true }); + }); + + it("folds its own reported inputs into its reported hash", () => { + const manifest = graphManifest(root); + expect(graphManifestHash(manifest.inputs)).toBe(manifest.manifestHash); + expect(manifest.inputs.configHash).toBe(manifest.configHash); + expect(manifest.inputs.grammarHash).toBe(manifest.grammarHash); + }); + + it("moves the manifest hash when only config content changes", () => { + const before = graphManifest(root); + // A field that affects resolution; a version bump deliberately does not. + writeFileSync(join(root, "package.json"), JSON.stringify({ name: "fixture", type: "module" })); + const after = graphManifest(root); + expect(after.configHash).not.toBe(before.configHash); + expect(after.manifestHash).not.toBe(before.manifestHash); + expect(graphManifestDiffersOnlyByConfig(after, before.manifestHash, before.configHash)).toBe(true); + }); + + const withInputs = (base: GraphManifest, patch: Partial): GraphManifest => { + const inputs = { ...base.inputs, ...patch }; + return { + manifestHash: graphManifestHash(inputs), + configHash: inputs.configHash, + grammarHash: inputs.grammarHash, + inputs, + }; + }; + + it("refuses every engine-identity input as config drift", () => { + const stored = graphManifest(root); + const drifted = withInputs(stored, { configHash: `${stored.configHash}0` }); + // The config-only baseline this fixture varies from. + expect(graphManifestDiffersOnlyByConfig(drifted, stored.manifestHash, stored.configHash)).toBe(true); + + for (const patch of [ + { db: stored.inputs.db + 1 }, + { compiler: "9.9.9" }, + { extractor: "extractor-next" }, + { resolver: "resolver-next" }, + { corpusPolicyHash: "0".repeat(64) }, + { grammarHash: "0".repeat(64) }, + ]) { + const current = withInputs(drifted, patch); + expect( + graphManifestDiffersOnlyByConfig(current, stored.manifestHash, stored.configHash), + `${Object.keys(patch)[0]} must not read as config drift`, + ).toBe(false); + } + }); + + it("fails closed without a stored manifest or config hash", () => { + const stored = graphManifest(root); + const current = withInputs(stored, { configHash: `${stored.configHash}0` }); + expect(graphManifestDiffersOnlyByConfig(current, undefined, stored.configHash)).toBe(false); + expect(graphManifestDiffersOnlyByConfig(current, stored.manifestHash, undefined)).toBe(false); + }); + + it("reports no drift when the stored manifest already matches", () => { + const stored = graphManifest(root); + expect(graphManifestDiffersOnlyByConfig(stored, stored.manifestHash, stored.configHash)).toBe(false); + }); +}); diff --git a/src/graph/__tests__/publish-known-gaps.test.ts b/src/graph/__tests__/publish-known-gaps.test.ts new file mode 100644 index 00000000..1f7c653b --- /dev/null +++ b/src/graph/__tests__/publish-known-gaps.test.ts @@ -0,0 +1,79 @@ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { GRAPH_CORPUS_LIMITS } from "../corpus-policy.js"; +import { rebuildGraph } from "../maintenance.js"; +import { inspectGraphStatus } from "../status.js"; + +const roots: string[] = []; + +afterEach(() => { + for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }); +}); + +function temporaryRoot(): string { + const root = mkdtempSync(join(tmpdir(), "mex-graph-publish-gaps-")); + roots.push(root); + return root; +} + +function write(root: string, path: string, source: string): void { + const absolute = join(root, path); + mkdirSync(dirname(absolute), { recursive: true }); + writeFileSync(absolute, source, "utf8"); +} + +/** A syntactically valid module far above the per-file ceiling. */ +function oversizedModule(): string { + const filler = `// ${"x".repeat(120)}\n`; + return "export const oversized = true;\n" + + filler.repeat(Math.ceil(GRAPH_CORPUS_LIMITS.maxSourceFileBytes / filler.length) + 1); +} + +describe("publishing a candidate with known gaps", () => { + it("publishes a graph when one file was skipped by the corpus policy", async () => { + const root = temporaryRoot(); + write(root, "src/alpha.ts", "export function alpha(): number {\n return beta();\n}\n"); + write(root, "src/beta.ts", "export function beta(): number {\n return 1;\n}\n"); + write(root, "src/generated.ts", oversizedModule()); + + const result = await rebuildGraph(root, {}); + + // The gap is real and reported, and every other file still reaches a graph. + expect(result.filesIndexed).toBe(2); + expect(result.skipped).toEqual([expect.objectContaining({ + filePath: "src/generated.ts", + reason: "corpus-limit", + limit: "maxSourceFileBytes", + })]); + const status = await inspectGraphStatus({ projectRoot: root }); + expect(status.status).not.toBe("missing"); + expect(status.changes.total).toBe(0); + }, 60_000); + +}); + +describe("a failed publication explains itself", () => { + it("reports the diagnostics the error carries, not just its headline", async () => { + const { describeGraphMaintenanceFailure } = await import("../cli-graph.js"); + const { GraphMaintenanceError } = await import("../maintenance.js"); + const described = describeGraphMaintenanceFailure(new GraphMaintenanceError( + "GRAPH_CANDIDATE_INVALID", + "The isolated graph candidate validated as degraded; the live graph was not replaced.", + [ + { code: "GRAPH_SOURCE_FILE_SKIPPED", severity: "warning", message: "too large", path: "src/big.ts" }, + { code: "GRAPH_PARSE_DEGRADED", severity: "warning", message: "1 partial", remediation: [{ label: "Rebuild", command: "mex graph rebuild" }] }, + ], + )); + expect(described).toContain("Observed 2 diagnostic(s)"); + expect(described).toContain("GRAPH_SOURCE_FILE_SKIPPED [src/big.ts]"); + expect(described).toContain("GRAPH_PARSE_DEGRADED"); + expect(described).toContain("Next: mex graph rebuild"); + }); + + it("passes a plain error through unchanged", async () => { + const { describeGraphMaintenanceFailure } = await import("../cli-graph.js"); + expect(describeGraphMaintenanceFailure(new Error("boom"))).toBe("boom"); + }); +}); diff --git a/src/graph/__tests__/snapshot.test.ts b/src/graph/__tests__/snapshot.test.ts index 0e005f70..789091a1 100644 --- a/src/graph/__tests__/snapshot.test.ts +++ b/src/graph/__tests__/snapshot.test.ts @@ -575,7 +575,7 @@ describe("graph snapshot provenance", () => { const dbPath = join(root, "graph.db"); mkdirSync(join(root, "src"), { recursive: true }); writeFileSync(sourcePath, "export function stableSnapshot(): number { return 7; }\n"); - writeFileSync(packagePath, "{\"name\":\"before-staging\"}\n"); + writeFileSync(packagePath, "{\"name\":\"before-staging\",\"type\":\"module\"}\n"); const baseline = createGraphEngine({ rootDir: root, dbPath }); await baseline.build(); @@ -591,7 +591,9 @@ describe("graph snapshot provenance", () => { read: (absolutePath) => { const source = readFileSync(absolutePath, "utf8"); if (!changedConfig && absolutePath === sourcePath) { - writeFileSync(packagePath, "{\"name\":\"during-staging\"}\n"); + // `type` decides how a specifier resolves, so this changes the + // build inputs rather than only the file's bytes. + writeFileSync(packagePath, "{\"name\":\"before-staging\",\"type\":\"commonjs\"}\n"); changedConfig = true; } return source; diff --git a/src/graph/cli-agent.ts b/src/graph/cli-agent.ts index 57b957e1..9db5034f 100644 --- a/src/graph/cli-agent.ts +++ b/src/graph/cli-agent.ts @@ -1,7 +1,7 @@ import { existsSync, readFileSync } from "node:fs"; import { relative, resolve } from "node:path"; import { globSync } from "glob"; -import { graphManifest } from "./engine-impl.js"; +import { graphManifest, graphManifestDiffersOnlyByConfig } from "./engine-impl.js"; import type { GraphEngine, GraphNeighbor, IndexedFileInfo } from "./engine.js"; import type { SqliteDatabase } from "./db/sqlite.js"; import { isSupportedSourceFile, SUPPORTED_SOURCE_GLOB } from "./extraction/index.js"; @@ -16,7 +16,6 @@ import { BudgetLedger, estimateTokens, resolveOptions, resolveScopeOptions, SCHEMA_VERSION, type AgentOptions, } from "./agent-protocol.js"; import { identifierComponents, isLowValueGraphPath, planGraphQuery } from "./retrieval/query.js"; -import { GraphRebuildRequiredError } from "./errors.js"; import { loadFreshGraphReadSession, openImmutableGraphReadSessionSync, @@ -24,13 +23,21 @@ import { type GraphReadValidation, type LoadFreshGraphReadSessionOptions, } from "./read-session.js"; +import { GRAPH_SNAPSHOT_METADATA_KEY, parseGraphSnapshot } from "./snapshot.js"; import type { GraphStatus } from "../team/contracts/graph.js"; +import type { GraphReadDegradation } from "./status.js"; type QueryRelation = "who-calls" | "what-calls" | "where-defined"; interface AgentGraphSession { graph: GraphEngine; db: SqliteDatabase; + /** Absent for caller-injected sessions, which make no freshness claim. */ + degradations?: readonly GraphReadDegradation[]; + /** Indexed paths whose facts describe an older revision of the file. */ + driftedSources?: readonly string[]; + /** The status a degraded answer must declare; present only when drifted. */ + graphStatus?: GraphStatus; readIndexedSource?: (filePath: string) => string; validate?: () => GraphReadValidation; revalidateFreshness?: () => Promise; @@ -74,10 +81,13 @@ export function runImpact( const fileNodes = nodesForFile(session, rootDir, target); const roots = fileNodes.length > 0 ? fileNodes : resolveSymbol(session.graph, target); if (roots.length === 0) { + for (const record of configDriftRecords(session)) writeJson(write, record); writeJson(write, { type: "error", code: "TARGET_NOT_FOUND", target }); return; } + if (emitTargetSourceDrifted(session, write, target, roots)) return; if (fileNodes.length === 0 && roots.length > 1) { + for (const record of configDriftRecords(session)) writeJson(write, record); writeJson(write, { type: "error", code: "TARGET_AMBIGUOUS", target, candidates: roots.map(nodeRef) }); return; } @@ -119,7 +129,10 @@ export function runImpact( if (emittedNodes.length >= opts.maxNodes) { truncated = true; break; } const fact = factFor(session, entry.node.id, opts.detail, opts.fingerprint); if (!fact) continue; - const record: Rec = { type: "caller", depth: entry.depth, root: entry.root, ...agentFactFields(fact, opts) }; + // Reached by following call edges, which is exactly what drifted + // resolution can get wrong. + const record: Rec = markResolutionStale(session, + { type: "caller", depth: entry.depth, root: entry.root, ...agentFactFields(fact, opts) }); if (!ledger.tryAdd(record)) { truncated = true; break; } factRecords.push(record); emittedNodes.push(entry.node); @@ -127,14 +140,20 @@ export function runImpact( const sourceRecords = planSource(session, ledger, emittedNodes, rootDir, opts); - const affectedIds = [...new Set([...roots.map((node) => node.id), ...impacted.keys()])]; + // Grounding is keyed by node, so an excluded node must not reappear + // through it. Nodes omitted only by the returned-node cap still count. + const affectedIds = [...new Set([...roots.map((node) => node.id), ...impacted.keys()])] + .filter((id) => !isDriftedFile(session, session.graph.getNode(id)?.filePath)); const groundingRecords: Rec[] = []; for (const grounding of groundedFiles(session.db, affectedIds)) { const record: Rec = { type: "grounding", node: grounding.node_id, file: grounding.scaffold_file }; if (ledger.tryAdd(record)) groundingRecords.push(record); else truncated = true; } - emitAll(write, meta, [...headRecords, ...factRecords, ...sourceRecords, ...groundingRecords]); + emitAll(write, meta, [ + ...configDriftRecords(session), + ...headRecords, ...factRecords, ...sourceRecords, ...groundingRecords, + ]); write(JSON.stringify(summaryRecord(ctx, { matchedNodes: roots.length + impacted.size, returnedNodes: emittedNodes.length, @@ -164,10 +183,13 @@ export function runGraphQuery( if (nodes.length === 0) { if (relation === "who-calls" && emitUnresolvedCallers(session, write, target, opts)) return; + for (const record of configDriftRecords(session)) writeJson(write, record); writeJson(write, { type: "error", code: "TARGET_NOT_FOUND", target }); return; } + if (emitTargetSourceDrifted(session, write, target, nodes)) return; + // Preserve (queried target, result) pairs; dedupe by that pair, not by result id alone. const pairs: Array<{ targetId: string; node: GraphNode }> = []; const seen = new Set(); @@ -193,14 +215,20 @@ export function runGraphQuery( if (entries.length >= opts.maxNodes) { truncated = true; break; } const fact = factFor(session, pair.node.id, opts.detail, opts.fingerprint); if (!fact) continue; - const record: Rec = { type: "result", relation, target: pair.targetId, ...agentFactFields(fact, opts) }; + // `where-defined` returns the declaration itself and is independent of + // resolution; the call relations are edges and are not. + const base: Rec = { type: "result", relation, target: pair.targetId, ...agentFactFields(fact, opts) }; + const record: Rec = relation === "where-defined" ? base : markResolutionStale(session, base); if (!ledger.tryAdd(record)) { truncated = true; break; } entries.push({ record, node: pair.node }); } const sourceRecords = planSource(session, ledger, entries.map((e) => e.node), rootDir, opts); - emitAll(write, meta, [...entries.map((e) => e.record), ...sourceRecords]); + emitAll(write, meta, [ + ...configDriftRecords(session), + ...entries.map((e) => e.record), ...sourceRecords, + ]); write(JSON.stringify(summaryRecord(ctx, { matchedNodes: pairs.length, returnedNodes: entries.length, @@ -485,8 +513,14 @@ export function runGraphScope( // Source is the high-value payload. Give it 75% first, then let unused // flow/fact capacity spill back into deferred source records. const plannedFlowRecords = trustworthyFlows.flatMap((flow) => { - const record = scopeFlowRecord(flow, nodeById, opts.maxFlowSteps); - return record ? [record] : []; + const planned = scopeFlowRecord(flow, nodeById, opts.maxFlowSteps); + // A flow is a chain of resolved edges, so drifted compiler inputs can + // change where it goes even when every file in it is current. The label + // belongs on the emitted record, not on the planning envelope that + // carries it alongside its step count. + return planned + ? [{ ...planned, record: markResolutionStale(session, planned.record) }] + : []; }); const summaryTokenReserve = estimateTokens(summarySkeleton([])) + RESERVE_PAD; const sourceRecords: Rec[] = []; @@ -668,6 +702,12 @@ export function runGraphScope( ...(highPriorityEvidenceOmitted ? [`High-priority evidence omitted: ${omittedHighPrioritySourceFiles.length} source file(s), ${omittedHighPriorityFlowSteps} flow step(s).`] : []), + ...(isConfigDrifted(session) + ? ["Graph build configuration changed after this index was built; flows and other resolved relationships may be out of date."] + : []), + ...(isParseDegraded(session) + ? ["Some indexed files did not parse completely; this answer may be missing symbols they define."] + : []), ]; const status = returnedFiles.length === 0 && facts.length === 0 && flowRecords.length === 0 ? "no-match" @@ -690,6 +730,7 @@ export function runGraphScope( } emitAll(write, meta, [ + ...configDriftRecords(session), ...healthRecords, ...sourceRecords, ...flowRecords, @@ -736,6 +777,15 @@ export function runGraphGet( if (ledger.tryAdd(record)) errorRecords.push(record); else truncated = true; continue; } + // Present in the index, excluded from the answer. Saying so is not the + // same as saying the node does not exist. + if (isDriftedFile(session, node.filePath)) { + const record: Rec = { + type: "error", code: "NODE_SOURCE_DRIFTED", id, filePath: node.filePath, + }; + if (ledger.tryAdd(record)) errorRecords.push(record); else truncated = true; + continue; + } nodes.push(node); } const sourceRecords = planSource(session, ledger, nodes, rootDir, opts); @@ -743,7 +793,9 @@ export function runGraphGet( sourceRecords.flatMap((record) => (record.ranges as SourceRange[]).flatMap((range) => range.nodeIds)), ); - emitAll(write, meta, [...errorRecords, ...sourceRecords]); + // `get` returns declarations and their proven source bytes only, so the + // drift declaration appears without any record being marked stale. + emitAll(write, meta, [...configDriftRecords(session), ...errorRecords, ...sourceRecords]); write(JSON.stringify(summaryRecord(ctx, { matchedNodes: ids.length, returnedNodes: sourcedIds.size, @@ -1028,6 +1080,10 @@ function scopeFactRecord(fact: CompactFact, score: number, reasons: string[], op function factFor(session: AgentGraphSession, id: string, detail: DetailLevel, includeFingerprint: boolean): CompactFact | null { const fact = compactFact(session.graph, id, detail); + // One seam for every command: a fact about a file that has moved on is not + // returned at all, rather than returned with coordinates that no longer + // point at what they describe. + if (fact && isDriftedFile(session, fact.filePath)) return null; if (!fact || !includeFingerprint) return fact; const fingerprint = new FingerprintStore(session.db).get(id); return fingerprint ? { ...fact, fingerprint: serializeFingerprint(fingerprint) } : fact; @@ -1876,7 +1932,7 @@ function withAgentGraphSession( // them synchronous so protocol goldens and deterministic unit harnesses do // not acquire filesystem/Git behavior they did not request. if (deps.open) return runInjectedAgentSession(rootDir, deps.open, write, task); - if (mode === "stable") return runStableAgentSession(rootDir, write, task); + if (mode === "stable") return runScopeAgentSession(rootDir, write, task); return runFreshAgentSession(rootDir, deps as AgentCommandInternalDeps, write, task); } @@ -1899,7 +1955,24 @@ function runInjectedAgentSession( } } -function runStableAgentSession( +/** + * Scope's gate, which asks the same question as the targeted commands and + * tolerates a different answer. + * + * Scope owns a per-file freshness pass: it hashes every indexed file's live + * source, discards the graph-derived facts of the ones that moved, and + * re-admits them as text-only evidence. Drifted source is therefore something + * it handles rather than something it must refuse — which is why it does not + * adopt the exact-freshness handshake the targeted commands use. Binding it to + * that handshake would make one edited file refuse a whole retrieval that + * currently answers. + * + * What it could not do was tell a store built by other code apart from one + * whose build inputs moved underneath it. It now asks that question through the + * same classifier, and answers a config-drifted store labelled instead of + * demanding a rebuild. + */ +function runScopeAgentSession( rootDir: string, write: (line: string) => void, task: AgentSessionTask, @@ -1914,11 +1987,14 @@ function runStableAgentSession( } const opened = openImmutableGraphReadSessionSync(rootDir, dbPath); session = { ...opened }; - const storedManifest = session.db.prepare( - "SELECT value FROM project_metadata WHERE key = 'manifest_hash'", - ).get() as { value: string } | undefined; - if (storedManifest?.value !== graphManifest(resolve(rootDir)).manifestHash) { - throw new GraphRebuildRequiredError("The code graph build manifest is stale."); + const stored = storedManifestIdentity(session.db); + const current = graphManifest(resolve(rootDir)); + if (stored.manifestHash !== current.manifestHash) { + if (!graphManifestDiffersOnlyByConfig(current, stored.manifestHash, stored.configHash)) { + manifestUnavailable(write); + return; + } + session = { ...session, degradations: ["config-drift"] }; } task(session, (line) => pending.push(line)); const validation = session.validate?.() ?? { valid: true }; @@ -1951,12 +2027,18 @@ async function runFreshAgentSession( ...deps.__internal?.freshRead, dbPath, loadSession: true, + allowDegradedReads: true, }); if (!loaded.session) { - graphStatusUnavailable(write, loaded.graphStatus); + graphStatusUnavailable(write, loaded.graphStatus, undefined, loaded.configDriftTolerated); return; } - session = { ...loaded.session }; + session = { + ...loaded.session, + degradations: loaded.session.degradations, + driftedSources: loaded.session.driftedSources, + graphStatus: loaded.graphStatus, + }; try { task(session, (line) => pending.push(line)); } catch (error) { @@ -1981,18 +2063,227 @@ async function runFreshAgentSession( } } +/** + * True when this session's structural facts describe compiler inputs that have + * since changed. + * + * Definition and containment facts survive that — a symbol's file, range and + * body text are read from the file itself, and the source bytes returned with + * them are proven byte-identical to what was indexed. What does not survive is + * resolution: `paths`, `moduleResolution`, `references` and a package's `type` + * decide which declaration a reference binds to, so any fact reached by + * following an edge may name the wrong target. + */ +function isConfigDrifted(session: AgentGraphSession): boolean { + return session.degradations?.includes("config-drift") === true; +} + +/** + * True when some files in this store parsed partially or not at all. + * + * This is a different claim from config drift, and a weaker one. Every fact + * the store holds is still true; there are simply fewer of them than the + * repository contains, so an answer can be missing a caller or a definition + * that lives in a file the parser could not finish. Nothing is relabelled + * `stale` for it — the answer is incomplete, not out of date. + */ +function isParseDegraded(session: AgentGraphSession): boolean { + return session.degradations?.includes("parse-degraded") === true; +} + +/** + * The files this answer must leave out. + * + * A file that changed since indexing invalidates every coordinate the graph + * holds for it — a line range now points somewhere else, and a symbol may not + * exist any more. Scope answers around that by re-admitting the file as + * text-only evidence; a command that returns exact node coordinates has no + * such fallback, so it drops those files' facts and answers from the rest. + * + * Dropping them silently would be the dishonest half. The response names them. + */ +function driftedSourceFiles(session: AgentGraphSession): ReadonlySet { + return new Set(session.driftedSources ?? []); +} + +function isSourceDrifted(session: AgentGraphSession): boolean { + return session.degradations?.includes("source-drift") === true; +} + +function isDriftedFile(session: AgentGraphSession, filePath: string | undefined): boolean { + return typeof filePath === "string" && driftedSourceFiles(session).has(filePath); +} + +/** + * Report a target that resolved, but only into files this answer excluded. + * + * Returning an empty result would be true and useless: the symbol exists, the + * store simply cannot describe where it is any more. Say that instead, with + * the file to refresh. + */ +function emitTargetSourceDrifted( + session: AgentGraphSession, + write: (line: string) => void, + target: string, + nodes: readonly GraphNode[], +): boolean { + if (nodes.length === 0 || !nodes.every((node) => isDriftedFile(session, node.filePath))) return false; + for (const record of configDriftRecords(session)) writeJson(write, record); + writeJson(write, { + type: "error", + code: "TARGET_SOURCE_DRIFTED", + target, + filePaths: [...new Set(nodes.map((node) => node.filePath))].sort(), + }); + return true; +} + +/** Mark one record as resolution-derived under config drift; otherwise unchanged. */ +function markResolutionStale(session: AgentGraphSession, record: Rec): Rec { + return isConfigDrifted(session) ? { ...record, stale: true } : record; +} + +/** + * The response-level declaration that this answer came from a drifted store. + * + * Emitted only when drifted, so a fresh response is byte-identical to what it + * was before degraded reads existed. It carries the same recovery command the + * refusal used to carry, as a record rather than an exception. + */ +function configDriftRecords(session: AgentGraphSession): Rec[] { + const degradations = [...(session.degradations ?? [])].sort(); + if (degradations.length === 0) return []; + // Scope classifies from the store's own manifest and has no inspection to + // quote, so the record degrades to its fixed half rather than disappearing. + const status = session.graphStatus; + const diagnostics = status?.diagnostics ?? []; + const drifted = isConfigDrifted(session); + const incomplete = isParseDegraded(session); + const sourceDrifted = isSourceDrifted(session); + const reasonDiagnostic = drifted + ? diagnostics.find((entry) => entry.code === "GRAPH_SEMANTIC_INPUTS_CHANGED") + ?? diagnostics.find((entry) => entry.code === "GRAPH_BUILD_MANIFEST_CHANGED") + : sourceDrifted + ? diagnostics.find((entry) => entry.code === "GRAPH_SOURCE_CORPUS_MISMATCH") + : diagnostics.find((entry) => entry.code === "GRAPH_PARSE_DEGRADED"); + const changedPaths = [...new Set(diagnostics + .filter((entry) => entry.code === "GRAPH_SEMANTIC_INPUT_CHANGED") + .map((entry) => (entry as { path?: unknown }).path) + .filter((path): path is string => typeof path === "string"))].sort(); + const recoveryCommand = diagnostics + .flatMap((entry) => entry.remediation ?? []) + .find((entry) => entry.command)?.command + ?? "mex graph refresh"; + const parseHealth = status?.parseHealth; + const excluded = [...(session.driftedSources ?? [])].sort(); + return [{ + type: "status", + // Config drift makes the store out of date; an unfinished parse only makes + // it incomplete. Report the kind the status inspection actually reached. + graphStatus: drifted || sourceDrifted ? "stale" : "degraded", + reasons: degradations, + ...(reasonDiagnostic?.code ? { reasonCode: reasonDiagnostic.code } : {}), + message: reasonDiagnostic?.message + ?? (drifted + ? "Graph build configuration changed after this index was built." + : sourceDrifted + ? "Some indexed files changed after this index was built and were excluded." + : "Some files could not be parsed completely when this index was built."), + // Say which part of the answer each label applies to, rather than leaving + // the reader to guess how much of it to discard. + trusted: ["definitions", "containment", "source"], + ...(drifted ? { stale: ["resolution", "edges"] } : {}), + ...(incomplete + ? { + incomplete: ["files that did not parse completely"], + partialFiles: parseHealth?.partial ?? 0, + failedFiles: parseHealth?.failed ?? 0, + ...(parseHealth && parseHealth.failedPaths.length > 0 + ? { + failedPaths: [...parseHealth.failedPaths].sort(), + ...(parseHealth.failedPathsTruncated ? { failedPathsTruncated: true } : {}), + } + : {}), + } + : {}), + ...(excluded.length > 0 + ? { + // The complete set, because the answer was built by excluding + // exactly these. A caller can reproduce what was left out. + excludedFiles: excluded, + excludedFileCount: excluded.length, + } + : {}), + ...(changedPaths.length > 0 ? { changedInputs: changedPaths } : {}), + ...(recoveryCommand ? { recoveryCommand } : {}), + }]; +} + +/** The build identity a store recorded, preferring its snapshot over loose metadata. */ +function storedManifestIdentity(db: SqliteDatabase): { + manifestHash: string | undefined; + configHash: string | undefined; +} { + const metadata = (key: string): string | undefined => { + const row = db.prepare("SELECT value FROM project_metadata WHERE key = ?").get(key) as + { value?: unknown } | undefined; + return typeof row?.value === "string" ? row.value : undefined; + }; + const snapshot = parseGraphSnapshot(metadata(GRAPH_SNAPSHOT_METADATA_KEY) ?? null); + return { + manifestHash: snapshot?.manifestHash ?? metadata("manifest_hash"), + configHash: snapshot?.configHash ?? metadata("config_hash"), + }; +} + +/** + * One refusal shape for every command. Scope used to raise its own error code + * for the same condition the targeted commands reported as unavailable, which + * left two vocabularies for one state. + */ +function manifestUnavailable(write: (line: string) => void): void { + writeJson(write, { + type: "error", + code: "GRAPH_UNAVAILABLE", + graphStatus: "rebuild_required", + reasonCode: "GRAPH_BUILD_MANIFEST_CHANGED", + message: "The graph was built by a different indexing engine; no graph-derived result was returned.", + recoveryCommand: "mex graph rebuild", + }); +} + +/** + * Diagnostics that describe drifted build configuration. + * + * When engine identity still reproduces, the read gate excuses all of these, + * so none of them can be the reason a read was refused. Naming one anyway + * sends the reader to revert a dependency bump that was never the blocker. + */ +const CONFIG_DRIFT_DIAGNOSTIC_CODES = new Set([ + "GRAPH_BUILD_MANIFEST_CHANGED", + "GRAPH_SEMANTIC_INPUTS_CHANGED", + "GRAPH_SEMANTIC_INPUT_CHANGED", +]); + function graphStatusUnavailable( write: (line: string) => void, status: GraphStatus, validation?: GraphReadValidation, + configDriftTolerated = false, ): void { + const blocking = configDriftTolerated + ? status.diagnostics.filter((entry) => !CONFIG_DRIFT_DIAGNOSTIC_CODES.has(entry.code)) + : status.diagnostics; + // Fall back to the full list only when config drift was the whole story and + // something else — a race, a mid-read change — still refused the read. + const candidates = blocking.length > 0 ? blocking : status.diagnostics; const diagnostic = validation?.code ? status.diagnostics.find((entry) => entry.code === validation.code) - : [...status.diagnostics].reverse().find((entry) => entry.severity !== "info") - ?? status.diagnostics.at(-1); + : [...candidates].reverse().find((entry) => entry.severity !== "info") + ?? candidates.at(-1); const graphStatus = validation && status.status === "fresh" ? "degraded" : status.status; const recoveryCommand = diagnostic?.remediation?.find((entry) => entry.command)?.command - ?? status.diagnostics.flatMap((entry) => entry.remediation ?? []) + ?? candidates.flatMap((entry) => entry.remediation ?? []) .find((entry) => entry.command)?.command; writeJson(write, { type: "error", @@ -2145,12 +2436,15 @@ function emitUnresolvedCallers( fromNode: row.from_node_id, ...(row.receiver === null ? {} : { receiver: row.receiver }), ...(row.qualifier === null ? {} : { qualifier: row.qualifier }), + // An unresolved reference is a resolution outcome, so drifted compiler + // inputs are the most likely reason this row exists at all. + ...(isConfigDrifted(session) ? { stale: true } : {}), }; if (!ctx.ledger.tryAdd(record)) { truncated = true; break; } records.push(record); } - emitAll(write, ctx.meta, records); + emitAll(write, ctx.meta, [...configDriftRecords(session), ...records]); write(JSON.stringify(summaryRecord(ctx, { matchedNodes: matched, // No node was returned: these are call sites, not declarations. diff --git a/src/graph/cli-graph.ts b/src/graph/cli-graph.ts index 04d9cb64..e294a9cd 100644 --- a/src/graph/cli-graph.ts +++ b/src/graph/cli-graph.ts @@ -108,6 +108,37 @@ function printDeclinedInputs(declined: GraphRefreshResult["declinedInputs"]): vo /** Human output stays bounded; `--json` carries the complete list. */ const MAX_SKIPPED_PATHS_SHOWN = 10; +/** + * Explain a maintenance failure with the observation that caused it. + * + * A failed publication used to print one sentence naming the status it + * refused, and discard the diagnostics the error carries. That left a user + * with a long build, no graph, and no way to learn which file was responsible + * or what was skipped along the way. + */ +export function describeGraphMaintenanceFailure(error: unknown): string { + const message = error instanceof Error ? error.message : String(error); + if (!(error instanceof GraphMaintenanceError) || error.diagnostics.length === 0) return message; + const lines = [message]; + const shown = error.diagnostics.slice(0, MAX_DIAGNOSTICS_SHOWN); + lines.push(`Observed ${error.diagnostics.length} diagnostic(s):`); + for (const diagnostic of shown) { + const path = (diagnostic as { path?: unknown }).path; + const where = typeof path === "string" ? ` [${path}]` : ""; + lines.push(` ${diagnostic.severity.toUpperCase()} ${diagnostic.code}${where}: ${diagnostic.message}`); + } + const omitted = error.diagnostics.length - shown.length; + if (omitted > 0) lines.push(` …and ${omitted} more`); + const command = error.diagnostics + .flatMap((diagnostic) => diagnostic.remediation ?? []) + .find((action) => action.command)?.command; + if (command) lines.push(`Next: ${command}`); + if (error.recoveryPath) lines.push(`Previous index retained at: ${error.recoveryPath}`); + return lines.join("\n"); +} + +const MAX_DIAGNOSTICS_SHOWN = 20; + function printStatus(status: GraphStatus): void { const branch = status.currentRepo.branch ?? "detached/no branch"; const head = status.currentRepo.head?.slice(0, 12) ?? "no HEAD"; diff --git a/src/graph/config-identity.ts b/src/graph/config-identity.ts new file mode 100644 index 00000000..59903449 --- /dev/null +++ b/src/graph/config-identity.ts @@ -0,0 +1,156 @@ +import ts from "typescript"; + +/** + * What a config file contributes to graph extraction, and nothing else. + * + * The build manifest folds the content of every `package.json`, `tsconfig` and + * `jsconfig` in the repository. Hashing their bytes made a dependency version, + * an npm script, an author field or a reindent look identical to a change in + * how modules resolve, and each of those invalidated the whole index. + * + * This projects each file down to the fields that actually decide what the + * compiler resolves and which files it reads. Everything else is dropped, so + * changing it cannot invalidate anything. + * + * **The hazard runs the other way.** Over-hashing is noisy; under-hashing is + * wrong, and silently — a field that affects extraction but is missing here + * would let a stale index read as current with nothing to say otherwise. Every + * field below is covered by a test asserting it still invalidates, and + * anything this cannot parse falls back to its exact bytes. + */ + +/** `compilerOptions` entries that change which declaration a reference binds to. */ +const SIGNIFICANT_COMPILER_OPTIONS = Object.freeze([ + "allowJs", + "baseUrl", + "checkJs", + "jsx", + "module", + "moduleResolution", + "paths", + "target", +] as const); + +/** Top-level tsconfig/jsconfig entries that change which files are in a program. */ +const SIGNIFICANT_TSCONFIG_FIELDS = Object.freeze([ + "exclude", + "extends", + "files", + "include", + "references", +] as const); + +/** `package.json` entries that change module resolution or project layout. */ +const SIGNIFICANT_PACKAGE_FIELDS = Object.freeze([ + "exports", + "imports", + "type", + "workspaces", +] as const); + +/** Dependency maps whose **names** shape resolution; their versions do not. */ +const DEPENDENCY_FIELDS = Object.freeze([ + "dependencies", + "devDependencies", + "optionalDependencies", + "peerDependencies", +] as const); + +export type GraphConfigKind = "package" | "tsconfig" | "unknown"; + +/** Classify by file name; the corpus policy only admits these three shapes. */ +export function graphConfigKind(path: string): GraphConfigKind { + const name = path.slice(path.lastIndexOf("/") + 1); + if (name === "package.json") return "package"; + if (/^(?:ts|js)config.*\.json$/u.test(name)) return "tsconfig"; + return "unknown"; +} + +/** + * A canonical string standing for everything in this file that can change the + * graph. + * + * Falls back to the exact source whenever the projection cannot be trusted: + * an unparseable file, an unrecognized name, or a document that is not a JSON + * object. Failing towards over-invalidation keeps an unreadable config from + * quietly meaning "nothing changed". + */ +export function graphConfigIdentity(path: string, source: string): string { + const kind = graphConfigKind(path); + if (kind === "unknown") return source; + const parsed = parseJsonWithComments(path, source); + if (parsed === undefined) return source; + const projected = kind === "package" ? projectPackage(parsed) : projectTsconfig(parsed); + return projected === undefined ? source : canonicalize(projected); +} + +/** tsconfig is JSON with comments and trailing commas; byte comparison is not parsing. */ +function parseJsonWithComments(path: string, source: string): unknown { + const result = ts.parseConfigFileTextToJson(path, source); + if (result.error || result.config === undefined) return undefined; + return result.config; +} + +function projectTsconfig(config: unknown): Record | undefined { + if (!isPlainObject(config)) return undefined; + const projected: Record = {}; + for (const field of SIGNIFICANT_TSCONFIG_FIELDS) { + if (field in config) projected[field] = config[field]; + } + const options = config.compilerOptions; + if (isPlainObject(options)) { + const significant: Record = {}; + for (const option of SIGNIFICANT_COMPILER_OPTIONS) { + if (option in options) significant[option] = options[option]; + } + if (Object.keys(significant).length > 0) projected.compilerOptions = significant; + } else if (options !== undefined) { + // A compilerOptions that is not an object is malformed; do not claim to + // have understood it. + return undefined; + } + return projected; +} + +function projectPackage(config: unknown): Record | undefined { + if (!isPlainObject(config)) return undefined; + const projected: Record = {}; + for (const field of SIGNIFICANT_PACKAGE_FIELDS) { + if (field in config) projected[field] = config[field]; + } + // A dependency's presence can change resolution; its version cannot change + // anything this graph extracts, and versions are what actually move. + const names = new Set(); + for (const field of DEPENDENCY_FIELDS) { + const entry = config[field]; + if (isPlainObject(entry)) for (const name of Object.keys(entry)) names.add(name); + else if (entry !== undefined) return undefined; + } + if (names.size > 0) projected.dependencyNames = [...names].sort(compareCodePoints); + return projected; +} + +/** + * Serialize with object keys in a fixed order so reordering or reindenting a + * file cannot change the result, while preserving array order, which is + * meaningful in `paths`, `include` and `references`. + */ +function canonicalize(value: unknown): string { + return JSON.stringify(normalize(value)); +} + +function normalize(value: unknown): unknown { + if (Array.isArray(value)) return value.map(normalize); + if (!isPlainObject(value)) return value; + const out: Record = {}; + for (const key of Object.keys(value).sort(compareCodePoints)) out[key] = normalize(value[key]); + return out; +} + +function isPlainObject(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function compareCodePoints(left: string, right: string): number { + return left < right ? -1 : left > right ? 1 : 0; +} diff --git a/src/graph/engine-impl.ts b/src/graph/engine-impl.ts index ced0fe3d..0a691cc8 100644 --- a/src/graph/engine-impl.ts +++ b/src/graph/engine-impl.ts @@ -14,7 +14,7 @@ import { } from "node:fs"; import { tmpdir } from "node:os"; import { dirname, isAbsolute, join, relative, resolve } from "node:path"; -import { toPosix } from "../paths.js"; +import { isSameResolvedPath, toPosix } from "../paths.js"; import type { BuildResult, DeclinedCompilerInput, GraphEngine, NodeSearchOptions, SkippedSourceFile, } from "./engine.js"; @@ -33,6 +33,7 @@ import { discoverBoundedGraphPaths, isPerFileCorpusLimitError, } from "./corpus-policy.js"; +import { graphConfigIdentity } from "./config-identity.js"; import { DB_SCHEMA_VERSION, markGraphReady, openGraphDatabase } from "./db/database.js"; import { GraphStore, @@ -274,6 +275,27 @@ export interface GraphManifest { manifestHash: string; configHash: string; grammarHash: string; + /** + * The exact inputs `manifestHash` was folded from. + * + * Kept alongside the hash so a reader can ask which *class* of input moved + * rather than only whether the fold changed. Config content is a build input + * that may shift under a usable index; the remaining entries are engine + * identity, and a difference in any of them means the store was written by + * code that no longer exists here. + */ + inputs: GraphManifestInputs; +} + +/** One fixed key order; the serialized form is the manifest hash preimage. */ +export interface GraphManifestInputs { + db: number; + compiler: string; + extractor: string; + resolver: string; + corpusPolicyHash: string; + grammarHash: string; + configHash: string; } class GraphEngineImpl implements GraphEngine { @@ -1504,7 +1526,7 @@ function readStableUtf8File( const resolvedAfter = realpathSync(sourcePath); const pathAfter = lstatSync(resolvedAfter); if (!sameFileIdentity(opened, after) - || resolvedAfter !== canonicalPath + || !isSameResolvedPath(resolvedAfter, canonicalPath) || !pathAfter.isFile() || pathAfter.isSymbolicLink() || !sameFileIdentity(opened, pathAfter)) { @@ -1756,7 +1778,7 @@ export function graphManifest(root: string): GraphManifest { const configSources = discoverGraphConfigSources(root); const configHash = configHashForSources(configSources); const grammarHash = grammarManifestHash(); - const manifestHash = sha256(JSON.stringify({ + const inputs: GraphManifestInputs = { db: DB_SCHEMA_VERSION, compiler: TYPESCRIPT_COMPILER_VERSION, extractor: CORPUS_EXTRACTOR_VERSION, @@ -1764,8 +1786,58 @@ export function graphManifest(root: string): GraphManifest { corpusPolicyHash: graphCorpusPolicyHash(root), grammarHash, configHash, + }; + return { + manifestHash: graphManifestHash(inputs), + configHash, + grammarHash, + inputs: Object.freeze(inputs), + }; +} + +/** + * Fold manifest inputs in one fixed order. + * + * Exported so a reader can re-fold *these* inputs with a different config hash + * and compare the result to a stored manifest. That substitution is the only + * supported way to ask whether a stored manifest and the current one differ by + * config alone, so the preimage must never be constructed anywhere else. + */ +export function graphManifestHash(inputs: GraphManifestInputs): string { + return sha256(JSON.stringify({ + db: inputs.db, + compiler: inputs.compiler, + extractor: inputs.extractor, + resolver: inputs.resolver, + corpusPolicyHash: inputs.corpusPolicyHash, + grammarHash: inputs.grammarHash, + configHash: inputs.configHash, })); - return { manifestHash, configHash, grammarHash }; +} + +/** + * True when a stored manifest is reproducible from the current engine identity + * and the config hash that store recorded — that is, when config content is the + * only manifest input that moved. + * + * This deliberately proves identity by reconstruction rather than by comparing + * a stored engine-identity field. A store written before this check existed + * records no such field, and those are exactly the stores that need to keep + * answering. Reconstruction also covers `corpusPolicyHash`, which no snapshot + * records at all, so an ignore-policy change cannot pass as config drift. + * + * Fails closed on anything it cannot prove: a store with no recorded config + * hash, or one whose manifest does not reproduce, is not config-drifted. + */ +export function graphManifestDiffersOnlyByConfig( + current: GraphManifest, + storedManifestHash: string | undefined, + storedConfigHash: string | undefined, +): boolean { + if (typeof storedManifestHash !== "string" || typeof storedConfigHash !== "string") return false; + if (storedManifestHash === current.manifestHash) return false; + if (storedConfigHash === current.configHash) return false; + return graphManifestHash({ ...current.inputs, configHash: storedConfigHash }) === storedManifestHash; } function discoverGraphConfigSources(root: string): Map { @@ -1804,9 +1876,18 @@ function discoverGraphConfigSources(root: string): Map { return new Map(configs); } +/** + * Identify config inputs by what they contribute to extraction, not by bytes. + * + * Hashing raw content made a dependency bump, an npm script or a reindent + * indistinguishable from a change to module resolution, and every one of them + * invalidated the index. `graphConfigIdentity` projects each file down to the + * fields that decide what the compiler resolves, and falls back to exact bytes + * for anything it cannot parse or recognize. + */ function configHashForSources(configSources: ReadonlyMap): string { return sha256(JSON.stringify([...configSources.entries()] - .map(([path, source]) => [path, sha256(source)]) + .map(([path, source]) => [path, sha256(graphConfigIdentity(path, source))]) .sort(([left], [right]) => compareCodePoints(left!, right!)))); } diff --git a/src/graph/maintenance.ts b/src/graph/maintenance.ts index 0c0ecb47..77a0dfa4 100644 --- a/src/graph/maintenance.ts +++ b/src/graph/maintenance.ts @@ -1875,19 +1875,43 @@ function assertRepairPublishableCandidate(status: GraphStatus): void { assertPublishableCandidate(status); } +/** + * Diagnostics that describe a gap the build made deliberately and completely. + * + * A file the corpus policy declined to read, or one that parsed partially, is + * a known hole in an otherwise complete candidate — the indexing, publication + * and freshness paths all agree it is not in the corpus. Refusing to publish + * over one of these throws away every other file's facts to punish a gap that + * a rebuild would reproduce exactly, which leaves the repository with no graph + * at all rather than a graph with a documented hole. + * + * Corpus-*wide* breaches and incomplete inspections are deliberately absent: + * those mean the observation itself is untrustworthy, not that one file is + * missing from a trustworthy one. + */ +const PUBLISHABLE_DEGRADED_CODES = new Set([ + "GRAPH_PARSE_DEGRADED", + "GRAPH_INDEX_HEAD_CHANGED", + // Added by the per-file skip path. Without it the publish gate discarded the + // candidate that path exists to produce. + "GRAPH_SOURCE_FILE_SKIPPED", + // Bounded companion of the above: emitted when more files were skipped than + // the diagnostic limit reports individually. + "GRAPH_SOURCE_DIAGNOSTICS_TRUNCATED", +]); + function assertPublishableCandidate(status: GraphStatus): void { if (status.status === "fresh") return; - const degradedOnlyByParse = status.status === "degraded" + const degradedOnlyByKnownGaps = status.status === "degraded" && status.changes.total === 0 && !status.changes.branchChanged && !status.changes.manifestChanged && !status.changes.configChanged && !status.changes.grammarChanged && status.diagnostics.every((diagnostic) => ( - diagnostic.severity !== "error" - && (diagnostic.code === "GRAPH_PARSE_DEGRADED" || diagnostic.code === "GRAPH_INDEX_HEAD_CHANGED") + diagnostic.severity !== "error" && PUBLISHABLE_DEGRADED_CODES.has(diagnostic.code) )); - if (degradedOnlyByParse) return; + if (degradedOnlyByKnownGaps) return; throw new GraphMaintenanceError( "GRAPH_CANDIDATE_INVALID", `The isolated graph candidate validated as ${status.status}; the live graph was not replaced.`, diff --git a/src/graph/read-session.ts b/src/graph/read-session.ts index f6787ade..97e2ebc7 100644 --- a/src/graph/read-session.ts +++ b/src/graph/read-session.ts @@ -10,6 +10,7 @@ import { statSync, } from "node:fs"; import { isAbsolute, relative, resolve } from "node:path"; +import { isSameResolvedPath } from "../paths.js"; import type { GraphStatus } from "../team/contracts/graph.js"; import { GRAPH_CORPUS_LIMITS, GraphCorpusLimitError } from "./corpus-policy.js"; import { openGraphDatabase } from "./db/database.js"; @@ -23,6 +24,7 @@ import { type GraphSidecarProbe, type InternalGraphFreshObservationToken, type InternalGraphStatusInspection, + type GraphReadDegradation, } from "./status.js"; import { GRAPH_SNAPSHOT_METADATA_KEY, @@ -54,12 +56,27 @@ export interface GraphFreshnessRevalidation extends GraphReadValidation { /** @internal A stable database session bound to one successful freshness observation. */ export interface InternalFreshGraphReadSession extends InternalGraphReadSession { graphStatus: GraphStatus; + /** + * The ways this store falls short of `fresh`, empty when it does not. + * + * It is bound exactly either way; these say what a reader must qualify when + * it reports the answer. + */ + degradations: readonly GraphReadDegradation[]; + /** Complete, sorted list of indexed paths that no longer match the tree. */ + driftedSources: readonly string[]; revalidateFreshness(): Promise; } export interface InternalFreshGraphReadResult { graphStatus: GraphStatus; session: InternalFreshGraphReadSession | null; + /** + * True when config content drifted under an engine identity that still + * reproduces. A caller explaining a refusal uses this to avoid naming the + * one input the gate excused. + */ + configDriftTolerated?: boolean; } interface ImmutableGraphReadHooks { @@ -73,6 +90,15 @@ interface ImmutableGraphReadHooks { export interface LoadFreshGraphReadSessionOptions { dbPath?: string; loadSession?: boolean; + /** + * Also adopt a store whose only unproven input is config content, reported + * as `config-drifted`. + * + * Opt-in, because the caller — not this loader — owns the obligation to say + * so in its output. A consumer that cannot label a degraded answer must not + * receive one. + */ + allowDegradedReads?: boolean; inspectObservation?: typeof inspectGraphStatusWithFreshObservation; inspectSidecars?: typeof inspectGraphSidecars; afterStatusInspection?: ( @@ -342,16 +368,27 @@ export async function loadFreshGraphReadSession( const inspectObservation = options.inspectObservation ?? inspectGraphStatusWithFreshObservation; const inspection = await inspectObservation({ projectRoot, dbPath }); await options.afterStatusInspection?.(inspection); - const { graphStatus, freshObservation } = inspection; - if (graphStatus.status !== "fresh" || options.loadSession === false) { - return { graphStatus, session: null }; + const { graphStatus } = inspection; + const degraded = graphStatus.status !== "fresh" + && options.allowDegradedReads === true + && (inspection.degradedObservation ?? null) !== null + ? inspection.degradedObservation! + : null; + const degradations: readonly GraphReadDegradation[] = degraded?.degradations ?? []; + const driftedSources: readonly string[] = degraded?.driftedSources ?? []; + const configDriftTolerated = inspection.configDriftTolerated === true; + const withTolerance = (result: InternalFreshGraphReadResult): InternalFreshGraphReadResult => + ({ ...result, configDriftTolerated }); + const freshObservation = degraded ? degraded.token : inspection.freshObservation; + if ((graphStatus.status !== "fresh" && !degraded) || options.loadSession === false) { + return withTolerance({ graphStatus, session: null }); } if (!freshObservation || sha256(freshObservation.snapshotRaw) !== freshObservation.snapshotHash) { - return unavailableFreshGraphReadResult( + return withTolerance(unavailableFreshGraphReadResult( graphStatus, "GRAPH_INDEX_READER_SNAPSHOT_CHANGED", "The fresh graph observation could not be bound to an exact snapshot; graph reads were skipped.", - ); + )); } let base: InternalGraphReadSession | null = null; @@ -372,21 +409,21 @@ export async function loadFreshGraphReadSession( || !indexedFiles || !snapshotMatchesIndexedFiles(snapshot, indexedFiles)) { base.close(); - return unavailableFreshGraphReadResult( + return withTolerance(unavailableFreshGraphReadResult( graphStatus, "GRAPH_INDEX_READER_SNAPSHOT_CHANGED", "The graph snapshot changed while immutable readers were opening; graph reads were skipped.", - ); + )); } const openedValidation = base.validate(); if (!openedValidation.valid) { base.close(); - return unavailableFreshGraphReadResult( + return withTolerance(unavailableFreshGraphReadResult( graphStatus, openedValidation.code ?? "GRAPH_INDEX_READER_OPEN_FAILED", openedValidation.message ?? "The graph changed or became unavailable while immutable readers were opening.", - ); + )); } const guardedStatus: GraphStatus = { ...graphStatus, diagnostics: [...graphStatus.diagnostics] }; @@ -395,14 +432,34 @@ export async function loadFreshGraphReadSession( const session: InternalFreshGraphReadSession = { ...ownedBase, graphStatus: guardedStatus, + degradations, + driftedSources, validate: () => ownedBase.validate(), revalidateFreshness: async () => { const before = session.validate(); const finalInspection = await inspectObservation({ projectRoot, dbPath }); - if (finalInspection.graphStatus.status !== "fresh" || !finalInspection.freshObservation) { + // Output is committed under the class it was labelled with. A store + // that changed class mid-read — drifted while being read as fresh, or + // repaired while being read as drifted — carries a label the buffered + // records no longer earn, so the response is discarded rather than + // relabelled after the fact. + const finalDegraded = finalInspection.degradedObservation ?? null; + const finalObservation = degradations.length > 0 + ? (finalDegraded + && sameDegradations(finalDegraded.degradations, degradations) + // A file that drifted after the answer was assembled would make a + // returned fact describe a revision that no longer exists, so the + // exact set has to hold for the whole read, not just its kinds. + && sameStringSets(finalDegraded.driftedSources, driftedSources) + ? finalDegraded.token + : null) + : finalInspection.graphStatus.status === "fresh" + ? finalInspection.freshObservation + : null; + if (!finalObservation) { return { valid: false, graphStatus: finalInspection.graphStatus }; } - if (!sameObservation(freshObservation, finalInspection.freshObservation)) { + if (!sameObservation(freshObservation, finalObservation)) { const changed = unavailableStatus( finalInspection.graphStatus, "GRAPH_INDEX_READER_SNAPSHOT_CHANGED", @@ -422,11 +479,11 @@ export async function loadFreshGraphReadSession( : { ...after, graphStatus: guardedStatus }; }, }; - return { graphStatus: guardedStatus, session }; + return withTolerance({ graphStatus: guardedStatus, session }); } catch (error) { base?.close(); const coded = graphReadError(error); - return unavailableFreshGraphReadResult(graphStatus, coded.code, coded.message); + return withTolerance(unavailableFreshGraphReadResult(graphStatus, coded.code, coded.message)); } } @@ -514,7 +571,7 @@ function readStableContainedSource(projectRoot: string, filePath: string): Buffe const resolvedAfter = realpathSync(absolutePath); const pathAfter = lstatSync(resolvedAfter); if (!sameFileIdentity(opened, after) - || resolvedAfter !== canonicalPath + || !isSameResolvedPath(resolvedAfter, canonicalPath) || !pathAfter.isFile() || pathAfter.isSymbolicLink() || !sameFileIdentity(opened, pathAfter)) { @@ -658,7 +715,7 @@ function bindDatabaseFile(dbPath: string, afterClose?: () => void): BoundDatabas const resolvedAfter = realpathSync(dbPath); const pathAfter = lstatSync(resolvedAfter); if (!opened.isFile() - || resolvedAfter !== dbPath + || !isSameResolvedPath(resolvedAfter, dbPath) || !pathAfter.isFile() || pathAfter.isSymbolicLink() || !sameFileIdentity(before, opened) @@ -757,6 +814,18 @@ function snapshotMatchesIndexedFiles( && snapshot.parseHealth.failed === failed; } +/** Order-independent equality; the producer sorts, this must not depend on it. */ +function sameDegradations( + left: readonly GraphReadDegradation[], + right: readonly GraphReadDegradation[], +): boolean { + return left.length === right.length && left.every((entry) => right.includes(entry)); +} + +function sameStringSets(left: readonly string[], right: readonly string[]): boolean { + return left.length === right.length && left.every((entry) => right.includes(entry)); +} + function sameObservation( left: InternalGraphFreshObservationToken, right: InternalGraphFreshObservationToken, diff --git a/src/graph/status.ts b/src/graph/status.ts index c4825da5..72caa8d6 100644 --- a/src/graph/status.ts +++ b/src/graph/status.ts @@ -12,6 +12,7 @@ import { statSync, } from "node:fs"; import { basename, dirname, isAbsolute, relative, resolve } from "node:path"; +import { isSameResolvedPath } from "../paths.js"; import { promisify } from "node:util"; import type { GraphParseHealth, @@ -35,7 +36,7 @@ import { createGraphSemanticInputLedger, discoverBoundedGraphPaths, } from "./corpus-policy.js"; -import { graphManifest } from "./engine-impl.js"; +import { graphManifest, graphManifestDiffersOnlyByConfig } from "./engine-impl.js"; import { isSupportedSourceFile } from "./extraction/grammars.js"; import { bandHashInts, decodeMinhash } from "./fingerprint.js"; import type { Fingerprint } from "./reconcile.js"; @@ -244,6 +245,25 @@ export interface InternalGraphFreshObservationToken { export interface InternalGraphStatusInspection { readonly graphStatus: GraphStatus; readonly freshObservation: InternalGraphFreshObservationToken | null; + /** + * @internal Exact identity for a store that would read `fresh` if its config + * inputs had not drifted. + * + * Optional so a caller-supplied status function, which cannot prove the + * distinction, keeps abstaining. Present only alongside a `stale` status: + * the store is bindable, but what it says about resolution is not current. + */ + readonly degradedObservation?: InternalGraphDegradedObservation | null; + /** + * @internal True when config content drifted under an engine identity that + * still reproduces — whether or not the store was servable. + * + * A refusal must name what is actually blocking it. Config drift is excused + * by the read gate, so when something else blocks the same store, reporting + * the config change as the reason sends a reader to revert an edit that was + * never the problem. + */ + readonly configDriftTolerated?: boolean; } interface FileRow { @@ -297,6 +317,38 @@ interface InspectionContext { maxChangedPaths: number; } +/** + * @internal A known, bounded way a bindable store falls short of `fresh`. + * + * `config-drift` — the compiler inputs that produced its resolution changed, + * so what it says about resolved references may be out of date. + * `parse-degraded` — some files parsed partially or not at all, so what it + * holds is incomplete. Its facts are still true; there are simply fewer of + * them than the repository contains. + * `source-drift` — some indexed files no longer match the working tree, so + * everything the store says about *those* files describes an older revision. + * Facts about every other file are unaffected. + * + * They make different claims and are reported separately. + */ +export type GraphReadDegradation = "config-drift" | "parse-degraded" | "source-drift"; + +/** @internal Exact identity plus the reasons the store is not provably fresh. */ +export interface InternalGraphDegradedObservation { + readonly token: InternalGraphFreshObservationToken; + readonly degradations: readonly GraphReadDegradation[]; + /** + * Every repository path whose indexed facts no longer describe the working + * tree, complete and sorted. + * + * Complete is the load-bearing word: a reader excludes these files and + * answers from the rest, so a partial list would let it serve a fact about + * a file that has moved on. The classification refuses whenever the change + * list was truncated, which is what makes this exhaustive. + */ + readonly driftedSources: readonly string[]; +} + interface FreshObservation { repo: RepoObservation; live: LiveSources; @@ -320,6 +372,8 @@ interface InspectionAttempt { status: GraphStatus; retry: boolean; freshObservation?: InternalGraphFreshObservationToken; + degradedObservation?: InternalGraphDegradedObservation; + configDriftTolerated?: boolean; } interface ClassifiedError { @@ -369,12 +423,16 @@ export async function inspectGraphStatusWithFreshObservation( return { graphStatus: inspected.status, freshObservation: inspected.freshObservation ?? null, + degradedObservation: inspected.degradedObservation ?? null, + configDriftTolerated: inspected.configDriftTolerated === true, }; } } return { graphStatus: lastAttempt!.status, freshObservation: null, + degradedObservation: null, + configDriftTolerated: lastAttempt!.configDriftTolerated === true, }; } @@ -913,8 +971,59 @@ async function inspectGraphStatusAttempt( changes: sourceChanges.changes, diagnostics, }); - if (status !== "fresh" || !snapshot || !snapshotRaw || !manifest) { - return finishDatabaseResult(result); + // A store whose only unproven input is config content is still an exact + // description of the source it indexed. Bind it like a fresh one so a + // reader can serve it labelled, and keep every other reason to distrust it + // — engine identity, source drift, branch, corpus digest, parse health, + // incomplete inspection — refusing exactly as before. + // Config content is forgivable on its own whenever engine identity still + // reproduces, independently of whether anything else also blocks the read. + // A caller that must explain a refusal needs that separately: config is + // then the input that was excused, never the reason. + const configDriftTolerated = snapshot !== undefined + && manifest !== null + && sourceChanges.changes.configChanged + && (snapshot.manifestHash === manifest.manifestHash + || graphManifestDiffersOnlyByConfig(manifest, snapshot.manifestHash, snapshot.configHash)); + // Everything except the store's own completeness must still hold. A + // corpus, branch, digest, grammar or engine-identity difference, an + // unfinished inspection, or a demanded rebuild all refuse exactly as + // before; the two entries below are the only shortfalls a bound read + // tolerates, and each is reported to whatever serves it. + // Drifted source is tolerable only while the complete list of drifted + // paths is known: a reader excludes exactly those files and answers from + // the rest, and a truncated list would let one through. The change-path + // ceiling therefore doubles as the bound on how much drift can still be + // served — past it, this is a stale index rather than a partial answer. + const sourceDrifted = sourceChanges.changes.total > 0; + const sourceDriftBounded = sourceDrifted && !sourceChanges.changes.truncated; + const boundable = !rebuildRequired + && !freshnessUnproven + && !sourceChanges.changes.branchChanged + && !sourceChanges.changes.grammarChanged + && (configDriftTolerated || !sourceChanges.changes.configChanged) + && (!sourceDrifted || sourceDriftBounded) + && (!sourceChanges.digestChanged || sourceDriftBounded); + const degradations: GraphReadDegradation[] = boundable + ? [ + ...(sourceChanges.changes.configChanged ? ["config-drift" as const] : []), + ...(parseDegraded ? ["parse-degraded" as const] : []), + ...(sourceDrifted ? ["source-drift" as const] : []), + ] + : []; + const driftedSources = sourceDriftBounded + ? [...new Set([ + ...sourceChanges.changes.added, + ...sourceChanges.changes.modified, + ...sourceChanges.changes.deleted, + ])].sort(compareCodePoints) + : []; + // `stale` and `degraded` are the only non-fresh kinds those shortfalls can + // produce here; any other kind was returned long before this point. + const bindable = status === "fresh" + || (boundable && degradations.length > 0 && (status === "stale" || status === "degraded")); + if (!bindable || !snapshot || !snapshotRaw || !manifest) { + return { ...finishDatabaseResult(result), configDriftTolerated }; } await context.options.internal?.beforeFreshValidation?.(attempt); @@ -927,12 +1036,21 @@ async function inspectGraphStatusAttempt( snapshotRaw, database, databaseIdentity: databaseFileIdentity(fileStat), - }); + }, degradations); if (validation.stable) { return { retry: false, status: result, - freshObservation: validation.freshObservation, + configDriftTolerated, + ...(degradations.length === 0 + ? { freshObservation: validation.freshObservation } + : { + degradedObservation: { + token: validation.freshObservation!, + degradations: Object.freeze([...degradations]), + driftedSources: Object.freeze([...driftedSources]), + }, + }), }; } const unstable = { @@ -1182,14 +1300,14 @@ function stabilizeDatabaseResult( // A missing/replaced database is handled as an unstable observation below. } if (sidecars.state === "clear" - && resolvedPath === dbPath + && isSameResolvedPath(resolvedPath, dbPath) && identityAfter === identityBefore) { return { retry: false, status: result }; } const diagnostics = [ ...(sidecars.state === "clear" ? [] : [sidecarDiagnostic(sidecars)]), observationRaceDiagnostic([ - resolvedPath !== dbPath + !isSameResolvedPath(resolvedPath, dbPath) ? "graph database path" : identityAfter === identityBefore ? "SQLite sidecars" @@ -1209,6 +1327,7 @@ function stabilizeDatabaseResult( async function validateFreshObservation( context: InspectionContext, before: FreshObservation, + degradations: readonly GraphReadDegradation[] = [], ): Promise { const firstSidecars = inspectGraphSidecars(before.database.canonicalPath); if (firstSidecars.state !== "clear") { @@ -1323,9 +1442,19 @@ async function validateFreshObservation( const changed: string[] = []; if (!sameRepoState(before.repo.state, repo.state)) changed.push("Git state"); + // Comparing the two observations to each other is the race check and always + // applies; comparing either to the stored snapshot is the freshness question + // the caller already answered. if (liveSourceIdentity(before.live) !== liveSourceIdentity(live)) changed.push("source corpus"); + // Two comparisons live here. Whether the two observations agree with each + // other is a race check and always applies. Whether they agree with the + // stored snapshot is a freshness check, already decided by the caller — for + // a config-drifted read it is the very condition being served, so repeating + // it here would report a race that did not happen. if (semanticInputIdentity(before.semantic) !== semanticInputIdentity(semantic) - || semantic.changedPaths.length > 0) changed.push("compiler semantic inputs"); + || (!degradations.includes("config-drift") && semantic.changedPaths.length > 0)) { + changed.push("compiler semantic inputs"); + } if (!sameManifest(before.manifest, manifest)) changed.push("graph manifest"); if (before.snapshotRaw !== snapshotRaw || before.databaseIdentity !== identity) changed.push("graph snapshot"); if (finalContained.database.canonicalPath !== contained.database.canonicalPath @@ -1594,7 +1723,7 @@ function readStableContainedUtf8File( const resolvedAfter = realpathSync(absolutePath); const pathAfter = lstatSync(resolvedAfter); if (databaseFileIdentity(opened) !== databaseFileIdentity(after) - || resolvedAfter !== canonicalPath + || !isSameResolvedPath(resolvedAfter, canonicalPath) || !pathAfter.isFile() || pathAfter.isSymbolicLink() || databaseFileIdentity(opened) !== databaseFileIdentity(pathAfter)) { diff --git a/src/paths.ts b/src/paths.ts index 68ad2662..d6b3e0b5 100644 --- a/src/paths.ts +++ b/src/paths.ts @@ -13,3 +13,29 @@ import { sep } from "node:path"; export function toPosix(p: string): string { return sep === "/" ? p : p.split(sep).join("/"); } + +/** + * Compare two already-resolved absolute paths for pointing at the same name. + * + * A re-resolved path is compared against the one a file was opened by, to + * prove the name still leads where it did. On Windows those two strings can + * differ in case alone — `realpathSync` preserves the casing it was handed, + * and a path that reached us through the TypeScript compiler host arrives + * lowercased — while naming the identical file on a case-insensitive volume. + * Comparing them as bytes rejected a file whose device, inode, size and + * timestamps all matched, and one such file failed an entire repository's + * build. + * + * This is a name comparison and nothing more. It never replaces the file + * identity checks around it, which are what actually detect a path that was + * repointed at different content. + */ +export function isSameResolvedPath(left: string | null, right: string | null): boolean { + if (left === null || right === null) return left === right; + if (left === right) return true; + // Case-insensitive volumes: Windows, and macOS by default. Comparing + // case-insensitively where the filesystem is case-sensitive would accept two + // genuinely different files, so it stays exact everywhere else. + if (process.platform !== "win32" && process.platform !== "darwin") return false; + return left.toLowerCase() === right.toLowerCase(); +} diff --git a/test/graph-cli-config-drift.test.ts b/test/graph-cli-config-drift.test.ts new file mode 100644 index 00000000..a950f9c6 --- /dev/null +++ b/test/graph-cli-config-drift.test.ts @@ -0,0 +1,254 @@ +import { mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import type { AgentCommandDeps } from "../src/graph/cli-agent.js"; +import { + runGraphGet, + runGraphQuery, + runGraphScope, + runImpact, +} from "../src/graph/cli-agent.js"; +import { openSqlite } from "../src/graph/db/sqlite.js"; +import { createGraphEngine } from "../src/graph/engine-impl.js"; +import { + GRAPH_SNAPSHOT_METADATA_KEY, + parseGraphSnapshot, + serializeGraphSnapshot, +} from "../src/graph/snapshot.js"; + +const roots: string[] = []; +type Rec = Record; + +afterEach(() => { + for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }); +}); + +interface Fixture { + root: string; + targetId: string; +} + +/** A two-package workspace, so a dependency bump touches only a config file. */ +async function fixture(): Promise { + const root = mkdtempSync(join(tmpdir(), "mex-graph-config-drift-cli-")); + roots.push(root); + mkdirSync(join(root, "packages", "api", "src"), { recursive: true }); + mkdirSync(join(root, "packages", "web", "src"), { recursive: true }); + writeFileSync(join(root, "package.json"), JSON.stringify({ + name: "fixture-root", private: true, workspaces: ["packages/*"], + dependencies: { "some-dependency": "1.0.0" }, + })); + writeFileSync(join(root, "tsconfig.json"), JSON.stringify({ + compilerOptions: { target: "ES2022", module: "NodeNext", moduleResolution: "NodeNext" }, + })); + for (const name of ["api", "web"]) { + writeFileSync(join(root, "packages", name, "package.json"), JSON.stringify({ + name: `@fixture/${name}`, version: "1.0.0", type: "module", + })); + } + writeFileSync(join(root, "packages", "api", "src", "index.ts"), + "export function handleRequest(path: string): string {\n return normalizePath(path);\n}\n" + + "export function normalizePath(path: string): string {\n return path.trim();\n}\n"); + writeFileSync(join(root, "packages", "web", "src", "index.ts"), + "import { handleRequest } from \"../../api/src/index.js\";\n" + + "export function renderPage(path: string): string {\n return handleRequest(path);\n}\n"); + const engine = createGraphEngine({ rootDir: root }); + await engine.build(); + const node = engine.searchNodes("normalizePath").find((entry) => entry.name === "normalizePath"); + if (!node) throw new Error("fixture node missing"); + engine.close(); + return { root, targetId: node.id }; +} + +/** + * Change a config field that genuinely affects extraction. + * + * A dependency *version* deliberately no longer registers: config inputs are + * identified by the fields that decide what the compiler resolves. `type` is + * one of those, so this is drift the graph must notice. + */ +function driftConfig(root: string): void { + writeFileSync(join(root, "package.json"), JSON.stringify({ + name: "fixture-root", private: true, type: "commonjs", workspaces: ["packages/*"], + dependencies: { "some-dependency": "1.0.0" }, + })); +} + +function breakEngineIdentity(root: string): void { + const db = openSqlite(join(root, ".mex", "graph.db")); + try { + const row = db.prepare("SELECT value FROM project_metadata WHERE key = ?") + .get(GRAPH_SNAPSHOT_METADATA_KEY) as { value: string }; + const snapshot = parseGraphSnapshot(row.value); + if (!snapshot) throw new Error("fixture has no snapshot"); + db.prepare("UPDATE project_metadata SET value = ? WHERE key = ?").run( + serializeGraphSnapshot({ ...snapshot, manifestHash: "0".repeat(64) }), + GRAPH_SNAPSHOT_METADATA_KEY, + ); + } finally { + db.close(); + } +} + +async function capture(command: (deps: AgentCommandDeps) => void | Promise): Promise { + const output: string[] = []; + await command({ write: (line) => output.push(line) }); + return output.map((line) => JSON.parse(line) as Rec); +} + +const statusRecord = (records: Rec[]): Rec | undefined => + records.find((record) => record.type === "status"); +const errorRecord = (records: Rec[]): Rec | undefined => + records.find((record) => record.type === "error"); +const ofType = (records: Rec[], type: string): Rec[] => + records.filter((record) => record.type === type); + +describe("graph reads after a config-only change", () => { + it("answers query, get and impact, labelled, instead of refusing", async () => { + const { root, targetId } = await fixture(); + driftConfig(root); + + const query = await capture((deps) => runGraphQuery("who-calls", "normalizePath", root, deps, {})); + expect(errorRecord(query)).toBeUndefined(); + expect(statusRecord(query)).toMatchObject({ + graphStatus: "stale", + reasons: ["config-drift"], + trusted: ["definitions", "containment", "source"], + stale: ["resolution", "edges"], + recoveryCommand: "mex graph refresh", + }); + const results = ofType(query, "result"); + expect(results.length).toBeGreaterThan(0); + expect(results.every((record) => record.stale === true)).toBe(true); + + const impact = await capture((deps) => runImpact("normalizePath", root, deps, {})); + expect(errorRecord(impact)).toBeUndefined(); + expect(statusRecord(impact)).toBeDefined(); + expect(ofType(impact, "caller").every((record) => record.stale === true)).toBe(true); + // A definition is not a resolution outcome and is not labelled. + expect(ofType(impact, "defines").every((record) => record.stale === undefined)).toBe(true); + + const got = await capture((deps) => runGraphGet([targetId], root, deps, { detail: "source" })); + expect(errorRecord(got)).toBeUndefined(); + expect(statusRecord(got)).toBeDefined(); + const sources = ofType(got, "source"); + expect(sources.length).toBeGreaterThan(0); + expect(sources.every((record) => record.stale === undefined)).toBe(true); + }); + + it("does not label where-defined, which no resolution produced", async () => { + const { root } = await fixture(); + driftConfig(root); + const records = await capture((deps) => runGraphQuery("where-defined", "normalizePath", root, deps, {})); + expect(statusRecord(records)).toBeDefined(); + expect(ofType(records, "result").every((record) => record.stale === undefined)).toBe(true); + }); + + it("answers scope, labelled, on the same gate", async () => { + const { root } = await fixture(); + driftConfig(root); + const records = await capture((deps) => runGraphScope("normalize request path", root, deps, {})); + expect(errorRecord(records)).toBeUndefined(); + expect(statusRecord(records)).toMatchObject({ + graphStatus: "stale", + reasons: ["config-drift"], + recoveryCommand: "mex graph refresh", + }); + const summary = records.find((record) => record.type === "summary"); + expect(summary?.warnings).toContainEqual(expect.stringContaining("build configuration changed")); + }); + + it("keeps answering scope when indexed source drifted, as it always did", async () => { + const { root } = await fixture(); + writeFileSync(join(root, "packages", "api", "src", "index.ts"), + "export function normalizePath(path: string): string {\n return path;\n}\n"); + const records = await capture((deps) => runGraphScope("normalize request path", root, deps, {})); + expect(errorRecord(records)).toBeUndefined(); + expect(statusRecord(records)).toBeUndefined(); + const health = records.find((record) => record.type === "health"); + expect(health?.staleFiles).toContain("packages/api/src/index.ts"); + }); + + it("still refuses every command when engine identity does not match", async () => { + const { root, targetId } = await fixture(); + driftConfig(root); + breakEngineIdentity(root); + const cases: Array<[string, Rec[]]> = [ + ["query", await capture((deps) => runGraphQuery("who-calls", "normalizePath", root, deps, {}))], + ["impact", await capture((deps) => runImpact("normalizePath", root, deps, {}))], + ["get", await capture((deps) => runGraphGet([targetId], root, deps, {}))], + ["scope", await capture((deps) => runGraphScope("normalize request path", root, deps, {}))], + ]; + for (const [name, records] of cases) { + expect(statusRecord(records), `${name} must not answer`).toBeUndefined(); + expect(errorRecord(records), `${name} must refuse`).toMatchObject({ type: "error" }); + } + }); + + it("labels scope flow records, not their planning envelope", async () => { + const { root } = await fixture(); + driftConfig(root); + const records = await capture((deps) => + runGraphScope("render page handle request normalize path", root, deps, { detail: "standard" })); + const flows = ofType(records, "flow"); + expect(flows.length).toBeGreaterThan(0); + expect(flows.every((record) => record.stale === true)).toBe(true); + expect(flows.every((record) => record.stepCount === undefined)).toBe(true); + }); + + it("answers around a source file that drifted alongside the config", async () => { + const { root } = await fixture(); + driftConfig(root); + writeFileSync(join(root, "packages", "api", "src", "index.ts"), + "export function normalizePath(path: string): string { return path; }"); + const records = await capture((deps) => runGraphQuery("where-defined", "renderPage", root, deps, {})); + expect(errorRecord(records)).toBeUndefined(); + expect(statusRecord(records)).toMatchObject({ + reasons: ["config-drift", "source-drift"], + excludedFiles: ["packages/api/src/index.ts"], + }); + // The answer comes from the file that did not move. + const results = ofType(records, "result"); + expect(results.length).toBeGreaterThan(0); + expect(results.every((record) => record.filePath === "packages/web/src/index.ts")).toBe(true); + }); + + it("emits nothing extra while the graph is fresh", async () => { + const { root } = await fixture(); + const records = await capture((deps) => runGraphQuery("who-calls", "normalizePath", root, deps, {})); + expect(statusRecord(records)).toBeUndefined(); + expect(records.every((record) => record.stale === undefined)).toBe(true); + }); + + it("does not touch the store while reading it drifted", async () => { + const { root } = await fixture(); + driftConfig(root); + const mexDir = join(root, ".mex"); + const before = readdirSync(mexDir).sort().map((name) => { + const path = join(mexDir, name); + return statSync(path).isFile() + ? { name, bytes: readFileSync(path).toString("base64") } + : { name, bytes: null }; + }); + await capture((deps) => runGraphQuery("who-calls", "normalizePath", root, deps, {})); + await capture((deps) => runGraphScope("normalize request path", root, deps, {})); + const after = readdirSync(mexDir).sort().map((name) => { + const path = join(mexDir, name); + return statSync(path).isFile() + ? { name, bytes: readFileSync(path).toString("base64") } + : { name, bytes: null }; + }); + expect(after).toEqual(before); + }); + + it("is byte-identical across repeated drifted reads", async () => { + const { root } = await fixture(); + driftConfig(root); + const once: string[] = []; + const twice: string[] = []; + await runGraphQuery("who-calls", "normalizePath", root, { write: (line) => once.push(line) }, {}); + await runGraphQuery("who-calls", "normalizePath", root, { write: (line) => twice.push(line) }, {}); + expect(twice).toEqual(once); + }); +}); diff --git a/test/graph-cli-freshness.test.ts b/test/graph-cli-freshness.test.ts index 9c2965c8..453ea23a 100644 --- a/test/graph-cli-freshness.test.ts +++ b/test/graph-cli-freshness.test.ts @@ -92,7 +92,7 @@ function expectUnavailableOnly(records: Record[], reason?: stri } describe("agent graph freshness-bound readers", () => { - it("makes Get, Query, and Impact abstain instead of pairing old nodes with changed live source", async () => { + it("never pairs an old node with changed live source, and excludes that file instead", async () => { const built = await fixture("mex-cli-stale-read-"); const fixed = new Date("2024-01-01T00:00:00.000Z"); const changed = built.original.replace("\"old\"", "\"new\""); @@ -108,10 +108,22 @@ describe("agent graph freshness-bound readers", () => { "stableFact", built.root, deps, { detail: "source" }, )); + // The invariant is that a node from a changed file is never described, + // and its live text never returned. The whole repository is one file here, + // so excluding it leaves nothing to answer from — but the response says + // which file it excluded rather than only that something was wrong. for (const records of [get, query, impact]) { - expectUnavailableOnly(records, "GRAPH_SOURCE_CORPUS_MISMATCH"); - expect(records[0]).toMatchObject({ graphStatus: "stale", recoveryCommand: "mex graph refresh" }); + const status = records.find((record) => record.type === "status"); + expect(status).toMatchObject({ + graphStatus: "stale", + reasons: ["source-drift"], + excludedFiles: ["src/service.ts"], + recoveryCommand: "mex graph refresh", + }); + expect(records.some((record) => record.type === "source")).toBe(false); + expect(records.some((record) => record.type === "result" || record.type === "defines")).toBe(false); expect(JSON.stringify(records)).not.toContain("return \\\"new\\\""); + expect(JSON.stringify(records)).not.toContain("return \\\"old\\\""); } }); diff --git a/test/graph-cli-parse-degraded.test.ts b/test/graph-cli-parse-degraded.test.ts new file mode 100644 index 00000000..65d5ad4b --- /dev/null +++ b/test/graph-cli-parse-degraded.test.ts @@ -0,0 +1,148 @@ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import type { AgentCommandDeps } from "../src/graph/cli-agent.js"; +import { runGraphQuery, runImpact } from "../src/graph/cli-agent.js"; +import { openSqlite } from "../src/graph/db/sqlite.js"; +import { createGraphEngine } from "../src/graph/engine-impl.js"; +import { + GRAPH_SNAPSHOT_METADATA_KEY, + parseGraphSnapshot, + serializeGraphSnapshot, +} from "../src/graph/snapshot.js"; +import { inspectGraphStatus } from "../src/graph/status.js"; + +const roots: string[] = []; +type Rec = Record; + +afterEach(() => { + for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }); +}); + +/** + * A store whose files parsed cleanly, then marked partial in place. + * + * Producing a genuinely partial parse from source is extractor-specific and + * would pin this test to whatever the parser currently tolerates. What is + * under test is the read gate's response to the recorded parse state, so the + * recorded state is what the fixture sets. + */ +async function parseDegradedFixture(): Promise { + const root = mkdtempSync(join(tmpdir(), "mex-graph-parse-degraded-")); + roots.push(root); + mkdirSync(join(root, "src"), { recursive: true }); + writeFileSync(join(root, "package.json"), JSON.stringify({ name: "fixture" })); + writeFileSync(join(root, "src", "a.ts"), + "export function alpha(): number {\n return beta();\n}\n" + + "export function beta(): number {\n return 1;\n}\n"); + writeFileSync(join(root, "src", "b.ts"), + "export function gamma(): number {\n return 2;\n}\n"); + const engine = createGraphEngine({ rootDir: root }); + await engine.build(); + engine.close(); + const db = openSqlite(join(root, ".mex", "graph.db")); + try { + db.prepare("UPDATE files SET parse_status = 'partial' WHERE path = ?").run("src/b.ts"); + // The snapshot records parse health too, and a snapshot that disagrees + // with the rows is corruption rather than degradation. Keep them in step, + // exactly as a real partial parse would have written them. + const row = db.prepare("SELECT value FROM project_metadata WHERE key = ?") + .get(GRAPH_SNAPSHOT_METADATA_KEY) as { value: string }; + const snapshot = parseGraphSnapshot(row.value); + if (!snapshot) throw new Error("fixture has no snapshot"); + db.prepare("UPDATE project_metadata SET value = ? WHERE key = ?").run( + serializeGraphSnapshot({ + ...snapshot, + parseHealth: { + ...snapshot.parseHealth, + ok: snapshot.parseHealth.ok - 1, + partial: snapshot.parseHealth.partial + 1, + }, + }), + GRAPH_SNAPSHOT_METADATA_KEY, + ); + } finally { + db.close(); + } + return root; +} + +async function capture(command: (deps: AgentCommandDeps) => void | Promise): Promise { + const output: string[] = []; + await command({ write: (line) => output.push(line) }); + return output.map((line) => JSON.parse(line) as Rec); +} + +const statusRecord = (records: Rec[]): Rec | undefined => + records.find((record) => record.type === "status"); + +describe("graph reads from a parse-degraded store", () => { + it("answers, and reports the answer as incomplete rather than stale", async () => { + const root = await parseDegradedFixture(); + expect((await inspectGraphStatus({ projectRoot: root })).status).toBe("degraded"); + + const records = await capture((deps) => runGraphQuery("who-calls", "beta", root, deps, {})); + expect(records.find((record) => record.type === "error")).toBeUndefined(); + const status = statusRecord(records); + expect(status).toMatchObject({ + graphStatus: "degraded", + reasons: ["parse-degraded"], + trusted: ["definitions", "containment", "source"], + partialFiles: 1, + failedFiles: 0, + }); + // Incomplete is not out of date: the facts this store holds are still true. + expect(status!.stale).toBeUndefined(); + const results = records.filter((record) => record.type === "result"); + expect(results.length).toBeGreaterThan(0); + expect(results.every((record) => record.stale === undefined)).toBe(true); + }); + + it("answers impact from the same store", async () => { + const root = await parseDegradedFixture(); + const records = await capture((deps) => runImpact("beta", root, deps, {})); + expect(records.find((record) => record.type === "error")).toBeUndefined(); + expect(statusRecord(records)).toMatchObject({ reasons: ["parse-degraded"] }); + expect(records.filter((record) => record.type === "caller").length).toBeGreaterThan(0); + }); + + it("reports both shortfalls when config also drifted", async () => { + const root = await parseDegradedFixture(); + writeFileSync(join(root, "package.json"), JSON.stringify({ name: "fixture", type: "commonjs" })); + const records = await capture((deps) => runGraphQuery("who-calls", "beta", root, deps, {})); + expect(records.find((record) => record.type === "error")).toBeUndefined(); + const status = statusRecord(records); + expect(status).toMatchObject({ + graphStatus: "stale", + reasons: ["config-drift", "parse-degraded"], + stale: ["resolution", "edges"], + partialFiles: 1, + }); + // Config drift is what makes an edge-derived record untrustworthy. + expect(records.filter((record) => record.type === "result") + .every((record) => record.stale === true)).toBe(true); + }); + + it("still refuses when the store is degraded for a reason it cannot bound", async () => { + const root = await parseDegradedFixture(); + const db = openSqlite(join(root, ".mex", "graph.db")); + try { + const row = db.prepare("SELECT value FROM project_metadata WHERE key = ?") + .get(GRAPH_SNAPSHOT_METADATA_KEY) as { value: string }; + const snapshot = parseGraphSnapshot(row.value); + if (!snapshot) throw new Error("fixture has no snapshot"); + // A different grammar is engine identity, not a bounded shortfall. + db.prepare("UPDATE project_metadata SET value = ? WHERE key = ?").run( + serializeGraphSnapshot({ ...snapshot, grammarHash: "0".repeat(64) }), + GRAPH_SNAPSHOT_METADATA_KEY, + ); + } finally { + db.close(); + } + const records = await capture((deps) => runGraphQuery("who-calls", "beta", root, deps, {})); + expect(records.find((record) => record.type === "error")) + .toMatchObject({ code: "GRAPH_UNAVAILABLE" }); + expect(statusRecord(records)).toBeUndefined(); + }); +}); diff --git a/test/graph-cli-source-drift.test.ts b/test/graph-cli-source-drift.test.ts new file mode 100644 index 00000000..1eb1198e --- /dev/null +++ b/test/graph-cli-source-drift.test.ts @@ -0,0 +1,148 @@ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import type { AgentCommandDeps } from "../src/graph/cli-agent.js"; +import { runGraphGet, runGraphQuery, runImpact } from "../src/graph/cli-agent.js"; +import { createGraphEngine } from "../src/graph/engine-impl.js"; + +const roots: string[] = []; +type Rec = Record; + +afterEach(() => { + for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }); +}); + +interface Fixture { + root: string; + stableId: string; + driftingId: string; +} + +/** + * Two files, one calling the other, so a single edit leaves a usable half. + */ +async function fixture(): Promise { + const root = mkdtempSync(join(tmpdir(), "mex-graph-source-drift-")); + roots.push(root); + mkdirSync(join(root, "src"), { recursive: true }); + writeFileSync(join(root, "package.json"), JSON.stringify({ name: "fixture" })); + writeFileSync(join(root, "src", "drifting.ts"), + "export function drifting(): number {\n return 1;\n}\n"); + writeFileSync(join(root, "src", "stable.ts"), + "export function stable(): number {\n return 2;\n}\n" + + "export function alsoStable(): number {\n return stable();\n}\n"); + const engine = createGraphEngine({ rootDir: root }); + await engine.build(); + const find = (name: string): string => { + const node = engine.searchNodes(name).find((entry) => entry.name === name); + if (!node) throw new Error(`fixture node ${name} missing`); + return node.id; + }; + const stableId = find("stable"); + const driftingId = find("drifting"); + engine.close(); + return { root, stableId, driftingId }; +} + +function editDriftingFile(root: string): void { + writeFileSync(join(root, "src", "drifting.ts"), + "export function drifting(): number {\n // moved\n return 3;\n}\n"); +} + +async function capture(command: (deps: AgentCommandDeps) => void | Promise): Promise { + const output: string[] = []; + await command({ write: (line) => output.push(line) }); + return output.map((line) => JSON.parse(line) as Rec); +} + +const statusRecord = (records: Rec[]): Rec | undefined => + records.find((record) => record.type === "status"); +const ofType = (records: Rec[], type: string): Rec[] => + records.filter((record) => record.type === type); + +describe("graph reads with drifted source files", () => { + it("answers from the files that did not change, and names the ones it left out", async () => { + const { root } = await fixture(); + editDriftingFile(root); + + const records = await capture((deps) => runGraphQuery("who-calls", "stable", root, deps, {})); + expect(records.find((record) => record.type === "error")).toBeUndefined(); + expect(statusRecord(records)).toMatchObject({ + graphStatus: "stale", + reasons: ["source-drift"], + excludedFiles: ["src/drifting.ts"], + excludedFileCount: 1, + }); + const results = ofType(records, "result"); + expect(results.length).toBeGreaterThan(0); + expect(results.every((record) => record.filePath === "src/stable.ts")).toBe(true); + }); + + it("excludes a node whose own file drifted rather than describing it", async () => { + const { root, driftingId } = await fixture(); + editDriftingFile(root); + + const records = await capture((deps) => runGraphGet([driftingId], root, deps, {})); + // Present in the index, excluded from the answer, and said so explicitly. + expect(records.find((record) => record.type === "error")).toMatchObject({ + code: "NODE_SOURCE_DRIFTED", + filePath: "src/drifting.ts", + }); + expect(ofType(records, "source")).toEqual([]); + }); + + it("returns a node from a file that did not drift", async () => { + const { root, stableId } = await fixture(); + editDriftingFile(root); + + const records = await capture((deps) => runGraphGet([stableId], root, deps, { detail: "source" })); + expect(records.find((record) => record.type === "error")).toBeUndefined(); + expect(statusRecord(records)).toMatchObject({ reasons: ["source-drift"] }); + expect(ofType(records, "source").length).toBeGreaterThan(0); + }); + + it("says a target resolved only into excluded files", async () => { + const { root } = await fixture(); + editDriftingFile(root); + + const records = await capture((deps) => runImpact("drifting", root, deps, {})); + // The symbol exists; what the store can no longer describe is where. An + // empty result would be true and useless. + expect(statusRecord(records)).toMatchObject({ + reasons: ["source-drift"], + excludedFiles: ["src/drifting.ts"], + }); + expect(records.find((record) => record.type === "error")) + .toMatchObject({ code: "TARGET_SOURCE_DRIFTED", filePaths: ["src/drifting.ts"] }); + }); + + it("treats a deleted indexed file as drift and keeps answering", async () => { + const { root } = await fixture(); + // A deleted indexed file is drift like any other and stays serveable. + rmSync(join(root, "src", "drifting.ts")); + const records = await capture((deps) => runGraphQuery("who-calls", "stable", root, deps, {})); + expect(records.find((record) => record.type === "error")).toBeUndefined(); + expect(statusRecord(records)).toMatchObject({ + reasons: ["source-drift"], + excludedFiles: ["src/drifting.ts"], + }); + }); + + it("emits nothing extra while every indexed file still matches", async () => { + const { root } = await fixture(); + const records = await capture((deps) => runGraphQuery("who-calls", "stable", root, deps, {})); + expect(statusRecord(records)).toBeUndefined(); + expect(records.every((record) => record.stale === undefined)).toBe(true); + }); + + it("is byte-identical across repeated drifted reads", async () => { + const { root } = await fixture(); + editDriftingFile(root); + const once: string[] = []; + const twice: string[] = []; + await runGraphQuery("who-calls", "stable", root, { write: (line) => once.push(line) }, {}); + await runGraphQuery("who-calls", "stable", root, { write: (line) => twice.push(line) }, {}); + expect(twice).toEqual(once); + }); +});