diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e2f49316..ab82d4fe4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelog +## [Unreleased] + +### Internal + +- The column-reader sweep was wrong in both directions and had been for as long as it existed. It answers "does this column have a consumer" for every column in the schema, and it is the tool this project leans on to catch a schema change that ships its writer and forgets its reader. Its idea of a write was any `field:` key anywhere, so `where: { ticketHash }` counted as writing `ticketHash`: all twelve columns it reported were a lookup key or a cron's discovery predicate being filtered on, every one a false alarm. And because it only ever reported a column that HAD a write, a column with no write at all was the one thing it could not see. It now understands which Prisma argument a key sits in, follows a payload assembled into a variable before the call, credits raw SQL against the mapped column name, and reports a column no code touches at all as its own category. Twelve false alarms became nineteen findings that each hold up, four of them worth acting on. The matcher underneath is pinned by a test that was checked by breaking it three ways. +- **Two columns have a reader and no writer.** The glucose unit preference is read across twenty-nine files, the dashboard and both exports and the doctor report and the FHIR resources among them, and no surface anywhere writes it, so an account cannot leave mg/dL. An import job's export date is described in the schema as parsed during unpack, is returned by the import status endpoint and is in the published contract, but the parser skips Apple's `ExportDate` element outright: the field has always been null. +- **Two columns have a writer and no reader.** The provider-health ledger records the HTTP status of a failed call and nothing reads it back, and the known-device ledger stamps a first-seen date that no query, page or export ever asks for. + ## [1.38.5] — 2026-09-03 The weekly backup finishes again, the dashboard stops scanning the two diff --git a/scripts/audit-column-readers.ts b/scripts/audit-column-readers.ts index fcbfce2ab..7b96782eb 100644 --- a/scripts/audit-column-readers.ts +++ b/scripts/audit-column-readers.ts @@ -1,5 +1,6 @@ /** - * Release-battery sweep: which Prisma columns are written but never read? + * Release-battery sweep: which Prisma columns have no consumer, and which have + * no writer at all? * * Run it as part of the deep-audit step, not in CI: * @@ -9,13 +10,13 @@ * * ## Why this is a script and not a test * - * The question "does this column have a reader anywhere in the tree" is a - * whole-program property, and the only cheap way to approximate it is grep. - * Grep is unsound in both directions here. Generic field names (`value`, - * `type`, `state`, `source`) match unrelated identifiers and read as consumed - * when they are not. Consumption through a select-less `findUnique` plus - * whole-row serialization reads as unconsumed when it is not. Raw SQL and - * dynamic selects fall outside the matcher entirely. + * The question "does this column have a consumer anywhere in the tree" is a + * whole-program property, and the only cheap way to approximate it is to read + * the source without type information. That approximation is unsound in both + * directions. Generic field names (`value`, `type`, `state`, `source`) match + * unrelated identifiers and read as consumed when they are not. Consumption + * through a select-less `findUnique` plus whole-row serialization reads as + * unconsumed when it is not. Dynamic selects fall outside the matcher. * * A gate built on that matcher would drift towards green and stop meaning * anything, which is exactly the failure this whole family of guards exists to @@ -27,6 +28,42 @@ * `src/__tests__/account-payload-consumer-guard.test.ts` and * `src/__tests__/measurement-backup-completeness.test.ts`. * + * ## What the matcher does, and where it stops + * + * Each file is masked once (comments, string bodies and regex literals blanked + * in place, `${…}` substitutions kept as code because `` `${row.field}` `` is a + * real read). Every `: {` / `: [` block is then bracket-matched and + * the characters inside it are tagged with the role that key gives them: + * + * - write — `data` `create` `createMany` `update` `updateMany` `upsert` + * `set` `connectOrCreate` + * - project — `select` `include` (a projection IS a read of the column) + * - query — `where` `orderBy` `cursor` `distinct` `having` `by` `omit` + * + * Nesting resolves innermost-wins, so the `where` inside an `upsert` is a + * query and the `data` beside it is a write. That distinction is the whole + * point of the rewrite: the previous matcher counted every `:` key + * anywhere as a write, so a column that is only ever FILTERED ON looked + * written-but-unread. All twelve findings it reported were that one mistake. + * + * Known limits, stated plainly so nobody mistakes a clean run for a proof: + * + * - Counts are per identifier, not per model. Two models with a `lastSeenAt` + * share one tally, so a read of either satisfies both. This is the same + * imprecision `GENERIC_NAMES` exists for, and it applies to every name. + * - `obj.field = …` counts as a write even when `obj` never reaches the + * database. Without it the settings layer, which assembles its update + * objects field by field, reads as columns nothing writes. + * - A non-Prisma object literal under a `data:` key (a chart series, a + * react-query result) is counted as a write. + * - A payload returned by a helper rather than declared beside its Prisma + * call is still invisible; the dataflow pass is one hop and one file. + * - Raw SQL is classified by keyword only: a literal shaped like INSERT or + * UPDATE…SET donates writes, one shaped like SELECT…FROM donates reads, + * for both the field name and its `@map` column name. + * - Relation attributes are read one line at a time; a `@relation(…)` split + * across lines would not register its `fields: […]`. + * * ## The annotation convention * * A pull request that adds a column, payload field, or preference contains its @@ -42,11 +79,17 @@ * becomes visible at the next release audit instead of never. */ import { readFileSync, readdirSync, statSync } from "node:fs"; -import { join } from "node:path"; +import { join, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; const ROOT = join(import.meta.dirname, ".."); const SCHEMA = join(ROOT, "prisma", "schema.prisma"); -const SRC = join(ROOT, "src"); +/** + * `src` is the application; `scripts` holds the one-shot maintenance tools, + * which are the other place a column legitimately gets written or read. Tests + * are excluded on purpose — a fixture is not a consumer. + */ +const SCAN_DIRS = ["src", "scripts"]; /** `generated` is the Prisma client: megabytes of noise, and never a consumer. */ const SKIP_DIRS = new Set([ @@ -60,7 +103,9 @@ const SKIP_DIRS = new Set([ /** * Names too generic for the matcher to say anything useful about. Reported in * their own section rather than dropped, so the sweep never claims a clean - * bill it cannot back up. + * bill it cannot back up. The suppression covers the consumer question only: + * a name that appears NOWHERE in the tree is still unwired, and that verdict + * does not get less true for a column called `state`. */ const GENERIC_NAMES = new Set([ "id", @@ -85,24 +130,24 @@ const GENERIC_NAMES = new Set([ "deletedAt", ]); -interface Field { +export interface Field { model: string; name: string; + /** `@map("…")` target, or the field name when it maps to itself. */ + column: string; annotation: "internal" | "pending-consumer" | null; annotationText: string | null; + /** `@default(…)`, `@updatedAt` or `@id`: the database fills it unaided. */ + dbManaged: boolean; + /** Relation properties that carry this scalar in their `fields: […]`. */ + relationBacked: string[]; } -function walk(dir: string, out: string[] = []): string[] { - for (const entry of readdirSync(dir)) { - if (SKIP_DIRS.has(entry)) continue; - const p = join(dir, entry); - if (statSync(p).isDirectory()) walk(p, out); - else if (entry.endsWith(".ts") || entry.endsWith(".tsx")) out.push(p); - } - return out; -} +// --------------------------------------------------------------------------- +// Schema +// --------------------------------------------------------------------------- -function parseSchema(schema: string): Field[] { +export function parseSchema(schema: string): Field[] { const models = new Set( [...schema.matchAll(/^model\s+([A-Za-z_]\w*)\s*\{/gm)].map((m) => m[1]), ); @@ -110,14 +155,19 @@ function parseSchema(schema: string): Field[] { for (const model of models) { const start = schema.indexOf(`model ${model} {`); + if (start === -1) continue; const end = schema.indexOf("\n}", start); const lines = schema.slice(start, end).split("\n").slice(1); + const scalars: Field[] = []; + /** scalar field name → relation properties that carry it. */ + const carriedBy = new Map(); + let pending: string[] = []; for (const rawLine of lines) { const line = rawLine.trim(); if (!line) continue; - if (line.startsWith("//") || line.startsWith("///")) { + if (line.startsWith("//")) { pending.push(line); continue; } @@ -131,11 +181,31 @@ function parseSchema(schema: string): Field[] { continue; } const [, name, type, isList] = m; + if (isList || models.has(type) || line.includes("@relation")) { + const carrier = /@relation\([^)]*\bfields:\s*\[([^\]]*)\]/.exec(line); + if (carrier) { + for (const raw of carrier[1].split(",")) { + const scalar = raw.trim(); + if (!scalar) continue; + carriedBy.set(scalar, [...(carriedBy.get(scalar) ?? []), name]); + } + } pending = []; continue; } - const comment = [...pending, line].join(" "); + + // Doc comments come in as `/// …` lines above the field, plus any + // trailing `// …` on the field line itself. Strip the markers and the + // declaration before reading an annotation out of them, or the reported + // reason comes back with the column's own Prisma type glued to its end. + const trailing = line.includes("//") + ? line.slice(line.indexOf("//")) + : ""; + const comment = [...pending, trailing] + .map((l) => l.replace(/^\/{2,3}\s?/, "").trim()) + .filter(Boolean) + .join(" "); let annotation: Field["annotation"] = null; let annotationText: string | null = null; const internal = /@internal:?\s*(.*)$/.exec(comment); @@ -149,36 +219,528 @@ function parseSchema(schema: string): Field[] { annotation = "internal"; annotationText = internal[1].trim(); } - fields.push({ model, name, annotation, annotationText }); + + const mapped = /@map\(\s*"([^"]*)"\s*\)/.exec(line); + scalars.push({ + model, + name, + column: mapped ? mapped[1] : name, + annotation, + annotationText, + dbManaged: + line.includes("@default(") || + line.includes("@updatedAt") || + /(?", + "\n", +]); + +export interface Masked { + /** Same length as the input; comments, string bodies and regexes blanked. */ + masked: string; + /** Every string and template chunk body, for the raw-SQL pass. */ + literals: string[]; } -function countUsage(text: string, field: string): Usage { - const esc = field.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); - const selects = text.match( - new RegExp(`(? { + for (let k = from; k < to; k++) if (out[k] !== "\n") out[k] = " "; }; + + /** Bottom frame is the file itself; each `` ` `` and `${` pushes one. */ + const stack: Array<{ tpl: boolean; depth: number }> = [ + { tpl: false, depth: 0 }, + ]; + let i = 0; + let lastSig = "\n"; + + while (i < n) { + const frame = stack[stack.length - 1]; + const c = src[i]; + + if (frame.tpl) { + if (c === "\\") { + blank(i, Math.min(i + 2, n)); + i += 2; + continue; + } + if (c === "`") { + out[i] = " "; + stack.pop(); + i += 1; + lastSig = '"'; + continue; + } + if (c === "$" && src[i + 1] === "{") { + blank(i, i + 2); + stack.push({ tpl: false, depth: 0 }); + i += 2; + lastSig = "("; + continue; + } + let end = i; + while ( + end < n && + src[end] !== "\\" && + src[end] !== "`" && + !(src[end] === "$" && src[end + 1] === "{") + ) + end++; + literals.push(src.slice(i, end)); + blank(i, end); + i = end; + continue; + } + + if (c === "/" && src[i + 1] === "/") { + const nl = src.indexOf("\n", i); + const end = nl === -1 ? n : nl; + blank(i, end); + i = end; + continue; + } + if (c === "/" && src[i + 1] === "*") { + const close = src.indexOf("*/", i + 2); + const end = close === -1 ? n : close + 2; + blank(i, end); + i = end; + continue; + } + if (c === "'" || c === '"') { + let end = i + 1; + while (end < n && src[end] !== c && src[end] !== "\n") { + if (src[end] === "\\") end++; + end++; + } + literals.push(src.slice(i + 1, end)); + blank(i, Math.min(end + 1, n)); + i = Math.min(end + 1, n); + lastSig = '"'; + continue; + } + if (c === "`") { + out[i] = " "; + stack.push({ tpl: true, depth: 0 }); + i += 1; + continue; + } + if (c === "/" && REGEX_PRECEDERS.has(lastSig)) { + let end = i + 1; + let inClass = false; + let closed = false; + while (end < n && src[end] !== "\n") { + const d = src[end]; + if (d === "\\") { + end += 2; + continue; + } + if (d === "[") inClass = true; + else if (d === "]") inClass = false; + else if (d === "/" && !inClass) { + end++; + closed = true; + break; + } + end++; + } + if (closed) { + while (end < n && /[a-z]/.test(src[end])) end++; + blank(i, end); + i = end; + lastSig = '"'; + continue; + } + // Never closed before the newline: it was a division after all. + } + if (c === "{") frame.depth++; + else if (c === "}") { + if (frame.depth === 0 && stack.length > 1) { + out[i] = " "; + stack.pop(); + i += 1; + lastSig = '"'; + continue; + } + frame.depth--; + } + if (!/\s/.test(c)) lastSig = c; + i += 1; + } + + return { masked: out.join(""), literals }; +} + +// --------------------------------------------------------------------------- +// Roles +// --------------------------------------------------------------------------- + +export const ROLE_NONE = 0; +export const ROLE_WRITE = 1; +export const ROLE_PROJECT = 2; +export const ROLE_QUERY = 3; + +const ROLE_KEYS: Record = { + data: ROLE_WRITE, + create: ROLE_WRITE, + createMany: ROLE_WRITE, + update: ROLE_WRITE, + updateMany: ROLE_WRITE, + upsert: ROLE_WRITE, + set: ROLE_WRITE, + connectOrCreate: ROLE_WRITE, + select: ROLE_PROJECT, + include: ROLE_PROJECT, + where: ROLE_QUERY, + orderBy: ROLE_QUERY, + cursor: ROLE_QUERY, + distinct: ROLE_QUERY, + having: ROLE_QUERY, + by: ROLE_QUERY, + omit: ROLE_QUERY, +}; + +const ROLE_KEY_RE = new RegExp( + `(? ({ … })) })` register as a write. + */ +function expressionExtent(text: string, from: number): number { + let depth = 0; + const limit = Math.min(text.length, from + EXTENT_CAP); + for (let i = from; i < limit; i++) { + const c = text[i]; + if (c === "(" || c === "[" || c === "{") depth++; + else if (c === ")" || c === "]" || c === "}") { + if (depth === 0) return i; + depth--; + } else if ((c === "," || c === ";") && depth === 0) return i; + } + return limit; +} + +/** The extent of a call's argument list, given the index of its `(`. */ +function callArguments(text: string, open: number): [number, number] { + let depth = 0; + for (let i = open; i < Math.min(text.length, open + EXTENT_CAP); i++) { + const c = text[i]; + if (c === "(" || c === "[" || c === "{") depth++; + else if (c === ")" || c === "]" || c === "}") { + depth--; + if (depth === 0) return [open, i]; + } + } + return [open, Math.min(text.length - 1, open + EXTENT_CAP)]; +} + +const WRITE_KEY_NAMES = Object.keys(ROLE_KEYS).filter( + (k) => ROLE_KEYS[k] === ROLE_WRITE, +); +/** `update: data` / `data: rows` — a payload handed over by name. */ +const WRITE_VALUE_RE = new RegExp( + `(?) { + const roles = new Uint8Array(masked.length); + spans.sort((a, b) => a[0] - b[0] || b[1] - a[1]); + for (const [from, to, role] of spans) roles.fill(role, from, to); + return roles; +} + +/** + * Tag every character with the role of the innermost `: …` value that + * encloses it. Outer values are painted first and inner ones paint over them, + * which is what makes `upsert: { where: {…}, create: {…} }` come out right. + * + * A second pass then follows payloads assembled before the call: a + * `const data = {…}` handed to `update:`, a `{ …spread }` merged into a + * `create:`, and the `rows.push({…})` builders that fill an array typed + * `Prisma.…Input[]`. All three are house style here — the no-mass-assignment + * rule makes field-by-field builders the norm — and without this pass most of + * the settings and import layer reads as columns nothing ever writes. + */ +export function roleMap(masked: string): Uint8Array { + const spans: Array<[number, number, number]> = []; + for (const m of masked.matchAll(ROLE_KEY_RE)) { + let from = m.index + m[0].length; + while (from < masked.length && /\s/.test(masked[from])) from++; + spans.push([from, expressionExtent(masked, from), ROLE_KEYS[m[1]]]); + } + + const roles = paint(masked, spans); + const sinks = new Set(); + for (const m of masked.matchAll(WRITE_VALUE_RE)) sinks.add(m[1]); + for (const m of masked.matchAll(TYPED_PAYLOAD_RE)) sinks.add(m[1]); + for (const m of masked.matchAll(SPREAD_RE)) + // The identifier ends the match, so its offset is the match's end minus + // its own length. The `d` flag would say this directly, but it is above + // the target this project compiles to. + if (roles[m.index + m[0].length - m[1].length] === ROLE_WRITE) + sinks.add(m[1]); + + for (const ident of sinks) { + const decl = new RegExp( + `(?; + +const bump = (t: Tally, k: string) => t.set(k, (t.get(k) ?? 0) + 1); + +/** + * Per-identifier counts, gathered in four passes over the whole tree rather + * than one pass per column: the schema has ~1500 scalars and re-scanning + * megabytes for each of them turns a review tool into a coffee break. + */ +export interface SourceIndex { + /** Keys and shorthand inside write payloads, plus `payload.field =` assignment. */ + writeKeys: Tally; + /** Keys inside `select` / `include` projections. */ + projectKeys: Tally; + /** Keys and shorthand inside filters and orderings. */ + queryKeys: Tally; + /** Shorthand destructuring that no Prisma argument claims. */ + plainShorthand: Tally; + /** `.field` property access, wherever it happens. */ + access: Tally; + /** Every identifier in code, plus every word in a string literal. */ + mentions: Tally; + /** Words inside INSERT / UPDATE…SET literals. */ + sqlWrite: Tally; + /** Words inside SELECT…FROM literals. */ + sqlRead: Tally; +} + +export function emptyIndex(): SourceIndex { + return { + writeKeys: new Map(), + projectKeys: new Map(), + queryKeys: new Map(), + plainShorthand: new Map(), + access: new Map(), + mentions: new Map(), + sqlWrite: new Map(), + sqlRead: new Map(), + }; +} + +const KEY_RE = /(? t.get(k) ?? 0; + const both = (t: Tally) => + get(t, field.name) + + (field.column === field.name ? 0 : get(t, field.column)); + + return { + writes: get(index.writeKeys, field.name) + both(index.sqlWrite), + reads: + get(index.projectKeys, field.name) + + get(index.access, field.name) + + get(index.plainShorthand, field.name) + + both(index.sqlRead), + queryUses: get(index.queryKeys, field.name), + mentions: both(index.mentions), + }; +} + +export type Category = + | "annotated-internal" + | "annotated-pending" + | "relation-backed" + | "unwired" + | "write-no-reader" + | "query-only" + | "read-no-writer" + | "generic" + | "consumed"; + +/** + * One column, one verdict. The order matters. An annotation is the maintainer + * speaking and outranks the matcher. `unwired` comes next because it is the + * only verdict here that does not depend on how the source is shaped — the + * identifier is simply absent — so neither a generic name nor a relation can + * excuse it. + */ +export function categorise(field: Field, usage: Usage): Category { + if (field.annotation === "internal") return "annotated-internal"; + if (field.annotation === "pending-consumer") return "annotated-pending"; + if (usage.mentions === 0) return "unwired"; + // Prisma writes the FK when the caller connects the relation, and reads it + // back when the caller selects the relation. Neither shows up under the + // scalar's own name, so the matcher cannot judge these at all. + if (field.relationBacked.length > 0) return "relation-backed"; + if (GENERIC_NAMES.has(field.name)) return "generic"; + if (usage.writes === 0) { + // `@default(…)` / `@updatedAt` / `@id`: the database is the writer. + if (field.dbManaged) return "consumed"; + return usage.reads > 0 || usage.queryUses > 0 + ? "read-no-writer" + : "unwired"; + } + if (usage.reads > 0) return "consumed"; + return usage.queryUses > 0 ? "query-only" : "write-no-reader"; +} + +// --------------------------------------------------------------------------- +// Driver +// --------------------------------------------------------------------------- + +function walk(dir: string, out: string[] = []): string[] { + for (const entry of readdirSync(dir)) { + if (SKIP_DIRS.has(entry)) continue; + const p = join(dir, entry); + if (statSync(p).isDirectory()) walk(p, out); + else if (/\.(ts|tsx|mts|mjs)$/.test(entry) && !/\.test\.tsx?$/.test(entry)) + out.push(p); + } + return out; } function main() { @@ -186,69 +748,109 @@ function main() { const modelFilter = args.includes("--model") ? args[args.indexOf("--model") + 1] : null; - const includeGeneric = args.includes("--all"); + const showAll = args.includes("--all"); const fields = parseSchema(readFileSync(SCHEMA, "utf8")).filter( (f) => !modelFilter || f.model === modelFilter, ); - const text = walk(SRC) - .map((f) => readFileSync(f, "utf8")) - .join("\n"); - - const findings: Array = []; - const generic: Array = []; - const pendingConsumers: Field[] = []; + const index = emptyIndex(); + for (const dir of SCAN_DIRS) + for (const file of walk(join(ROOT, dir))) + indexSource(readFileSync(file, "utf8"), index); + const buckets = new Map>(); for (const field of fields) { - if (field.annotation === "pending-consumer") pendingConsumers.push(field); - if (field.annotation) continue; - const usage = countUsage(text, field.name); - if (usage.reads > 0 || usage.selects > 0) continue; - if (usage.writes === 0) continue; - if (GENERIC_NAMES.has(field.name) && !includeGeneric) { - generic.push({ ...field, ...usage }); - continue; - } - findings.push({ ...field, ...usage }); + const usage = countUsage(index, field); + const category = categorise(field, usage); + buckets.set(category, [ + ...(buckets.get(category) ?? []), + { ...field, ...usage }, + ]); } + const bucket = (c: Category) => buckets.get(c) ?? []; const scope = modelFilter ? `model ${modelFilter}` : "every model"; console.log( - `Column-reader sweep over ${scope} — ${fields.length} scalar column(s).\n`, + `Column-reader sweep over ${scope} — ${fields.length} scalar column(s) ` + + `against ${SCAN_DIRS.join(" + ")}.\n`, ); - if (findings.length === 0) { - console.log("No unannotated write-only columns matched.\n"); - } else { - console.log( - `${findings.length} column(s) written but never read or selected — verify each by hand:\n`, - ); - for (const f of findings) { - console.log(` ${f.model}.${f.name} (writes: ${f.writes})`); - } + const section = ( + title: string, + rows: Array, + line: (f: Field & Usage) => string, + ) => { + if (rows.length === 0) return; + console.log(`## ${title} — ${rows.length}\n`); + for (const f of rows) console.log(` ${f.model}.${f.name} ${line(f)}`); console.log(""); - } + }; - if (pendingConsumers.length > 0) { - console.log( - `${pendingConsumers.length} column(s) marked @pending-consumer — check whether the issue closed without the consumer landing:\n`, + section( + "Written, and nothing anywhere consumes it", + bucket("write-no-reader"), + (f) => `(writes: ${f.writes})`, + ); + section( + "Neither written nor read — nothing in the tree wires this column", + bucket("unwired"), + (f) => `(column "${f.column}", mentions: ${f.mentions})`, + ); + section( + "Consumed only by a query clause — filtered or ordered on, never carried into JavaScript", + bucket("query-only"), + (f) => `(writes: ${f.writes}, query uses: ${f.queryUses})`, + ); + section( + "Read but never written by application code", + bucket("read-no-writer"), + (f) => `(reads: ${f.reads}, query uses: ${f.queryUses})`, + ); + section( + "Marked @pending-consumer — check whether the issue closed without the consumer landing", + bucket("annotated-pending"), + (f) => f.annotationText ?? "", + ); + + if (showAll) { + section( + "Marked @internal — permanently silenced, listed for review", + bucket("annotated-internal"), + (f) => f.annotationText ?? "", ); - for (const f of pendingConsumers) { - console.log(` ${f.model}.${f.name} ${f.annotationText ?? ""}`); - } - console.log(""); - } - - if (generic.length > 0) { - console.log( - `${generic.length} match(es) suppressed as too generic to judge (re-run with --all to see them).\n`, + section( + "Suppressed: name too generic for the matcher to judge", + bucket("generic"), + (f) => `(writes: ${f.writes}, reads: ${f.reads})`, + ); + section( + "Carried by a relation — Prisma writes and reads these under the relation's name", + bucket("relation-backed"), + (f) => `(via ${f.relationBacked.join(", ")})`, ); } + const quiet: Category[] = [ + "consumed", + "relation-backed", + "annotated-internal", + "generic", + ]; console.log( - "Expect false positives: raw SQL, dynamic selects, and whole-row serialization\n" + - "all read columns this matcher cannot see. Confirm a finding before acting on it.", + "Not listed: " + + quiet.map((c) => `${bucket(c).length} ${c}`).join(", ") + + (showAll ? "" : " (re-run with --all to see the last three)"), + ); + console.log( + "\nEvery finding above needs a human. Counts are per identifier rather than\n" + + "per model, whole-row serialization reads as unconsumed, and a non-Prisma\n" + + "`data:` literal reads as a write.", ); } -main(); +if ( + process.argv[1] && + resolve(process.argv[1]) === resolve(fileURLToPath(import.meta.url)) +) { + main(); +} diff --git a/src/__tests__/column-reader-audit-matcher.test.ts b/src/__tests__/column-reader-audit-matcher.test.ts new file mode 100644 index 000000000..735b01841 --- /dev/null +++ b/src/__tests__/column-reader-audit-matcher.test.ts @@ -0,0 +1,203 @@ +/** + * The column-reader sweep (`scripts/audit-column-readers.ts`) is a review tool, + * not a gate — the whole-program question it asks cannot be answered exactly + * without type information, so a human reads its output. But the matcher + * UNDERNEATH it can be pinned exactly, and until this file existed it was not: + * the sweep counted every `:` key as a write, so a column that is only + * ever filtered on (`where: { ticketHash } `) looked written-but-never-read. + * Every finding it produced was that one mistake, which is the failure mode a + * review tool cannot survive — an operator who has dismissed twelve false + * alarms dismisses the thirteenth finding too. + * + * So this file feeds the matcher a fixture whose right answers are known by + * construction and asserts the classification, one case per defect class. It + * does NOT assert anything about the real schema; that stays a human read. + */ +import { describe, expect, it } from "vitest"; +import { + categorise, + countUsage, + emptyIndex, + indexSource, + parseSchema, + type Category, +} from "../../scripts/audit-column-readers"; + +const SCHEMA = ` +model Widget { + id String @id @default(cuid()) + /// Looked up by, never written by application code. + lookupOnly String @map("lookup_only") + /// Stamped by a cron and used as that cron's discovery predicate. + backfilledAt DateTime? @map("backfilled_at") + /// Written, and nothing consumes it. + writeOnlyNote String? @map("write_only_note") + /// Written and read back. + consumedValue Int? @map("consumed_value") + /// Reached only through a select projection. + projectedValue Int? @map("projected_value") + /// Assembled into a payload variable before the Prisma call. + builtViaPayload String? @map("built_via_payload") + /// Pushed into a typed payload array. + builtViaPush String? @map("built_via_push") + /// Written and read by raw SQL only. + rawSqlColumn Int? @map("raw_sql_column") + /// Nothing anywhere touches this one. + orphanColumn String? @map("orphan_column") + /// @internal: bookkeeping for a cron; never surfaced. + internalColumn String? @map("internal_column") + /// @pending-consumer(#123) — the client reads this from a later build. + awaitedColumn String? @map("awaited_column") + ownerId String @map("owner_id") + + owner Owner @relation(fields: [ownerId], references: [id]) + + @@map("widgets") +} + +model Owner { + id String @id @default(cuid()) +} +`; + +/** + * Deliberately written the way the application writes: `where` filters beside + * `data` payloads, an `orderBy`, an `include`, a payload assembled into a + * variable, and a `.push` builder. Every one of those was a write to the old + * matcher. + */ +const SOURCE = ` +import { prisma, type Prisma } from "@/lib/db"; + +export async function findWidget(token: string) { + return prisma.widget.findUnique({ + where: { lookupOnly: token }, + orderBy: { backfilledAt: "desc" }, + include: { owner: true }, + select: { projectedValue: true }, + }); +} + +export async function claimWidget(id: string, note: string) { + const row = await prisma.widget.update({ + where: { id }, + data: { writeOnlyNote: note, consumedValue: 1, projectedValue: 2 }, + }); + return row.consumedValue; +} + +export async function sweepWidgets() { + const stale = await prisma.widget.findMany({ + where: { backfilledAt: null }, + }); + for (const widget of stale) { + await prisma.widget.update({ + where: { id: widget.id }, + data: { backfilledAt: new Date() }, + }); + } +} + +export async function savePreferences(input: { label?: string }) { + const updates: Prisma.WidgetUpdateInput = {}; + if (input.label !== undefined) updates.builtViaPayload = input.label; + await prisma.widget.update({ where: { id: "x" }, data: updates }); +} + +export async function seedWidgets(labels: string[]) { + const rows: Prisma.WidgetCreateManyInput[] = []; + for (const label of labels) { + rows.push({ ownerId: "o", lookupOnly: label, builtViaPush: label }); + } + await prisma.widget.createMany({ data: rows }); +} + +export async function bumpRawSql(id: string) { + await prisma.$executeRaw\` + UPDATE widgets SET raw_sql_column = raw_sql_column + 1 WHERE id = \${id} + \`; +} +`; + +function classify() { + const index = indexSource(SOURCE, emptyIndex()); + const out = new Map(); + for (const field of parseSchema(SCHEMA)) { + if (field.model !== "Widget") continue; + out.set(field.name, categorise(field, countUsage(index, field))); + } + return out; +} + +describe("column-reader audit matcher", () => { + const verdicts = classify(); + + it("does not mistake a `where:` filter for a write", () => { + // The exact defect: `lookupOnly` is filtered on, and written only inside a + // `rows.push({...})` builder. It must never come back as write-no-reader, + // the verdict the old matcher gave to all twelve of its findings. + expect(verdicts.get("lookupOnly")).not.toBe("write-no-reader"); + expect(verdicts.get("lookupOnly")).toBe("query-only"); + }); + + it("reads a cron's stamp-and-discover column as query-only, not unread", () => { + expect(verdicts.get("backfilledAt")).toBe("query-only"); + }); + + it("still reports a genuine write with no consumer", () => { + expect(verdicts.get("writeOnlyNote")).toBe("write-no-reader"); + }); + + it("reports a column nothing in the tree touches", () => { + expect(verdicts.get("orphanColumn")).toBe("unwired"); + }); + + it("counts property access and select projection as consumption", () => { + expect(verdicts.get("consumedValue")).toBe("consumed"); + expect(verdicts.get("projectedValue")).toBe("consumed"); + }); + + it("follows a payload assembled before the Prisma call", () => { + // `updates.builtViaPayload = …` then `data: updates`, and + // `rows.push({ builtViaPush })` then `data: rows`. Miss either and the + // whole settings and import layer reads as write-less columns. + const index = indexSource(SOURCE, emptyIndex()); + const fields = parseSchema(SCHEMA); + for (const name of ["builtViaPayload", "builtViaPush"]) { + const field = fields.find((f) => f.name === name)!; + expect(countUsage(index, field).writes).toBeGreaterThan(0); + } + }); + + it("credits raw SQL against the mapped column name", () => { + const index = indexSource(SOURCE, emptyIndex()); + const field = parseSchema(SCHEMA).find((f) => f.name === "rawSqlColumn")!; + expect(countUsage(index, field).writes).toBeGreaterThan(0); + }); + + it("lets an annotation outrank the matcher", () => { + expect(verdicts.get("internalColumn")).toBe("annotated-internal"); + expect(verdicts.get("awaitedColumn")).toBe("annotated-pending"); + }); + + it("leaves a relation's foreign key to the relation", () => { + // Prisma writes `ownerId` through `connect` and reads it back through + // `owner`; neither appears under the scalar's own name. + expect(verdicts.get("ownerId")).toBe("relation-backed"); + }); + + it("keeps `orderBy` and `include` out of the write set", () => { + const index = indexSource(SOURCE, emptyIndex()); + expect(index.writeKeys.get("orderBy") ?? 0).toBe(0); + expect(index.writeKeys.get("include") ?? 0).toBe(0); + expect(index.queryKeys.get("backfilledAt") ?? 0).toBeGreaterThan(0); + }); + + it("masks comments and string bodies out of the code it reads", () => { + const index = indexSource( + `const s = "data: { ghostColumn: 1 }";\n// data: { ghostColumn: 2 }\n`, + emptyIndex(), + ); + expect(index.writeKeys.get("ghostColumn") ?? 0).toBe(0); + }); +});