diff --git a/scripts/check-dead-exports.test.ts b/scripts/check-dead-exports.test.ts new file mode 100644 index 000000000..6d02b7594 --- /dev/null +++ b/scripts/check-dead-exports.test.ts @@ -0,0 +1,414 @@ +import { spawnSync } from "node:child_process"; +import { + existsSync, + readdirSync, + readFileSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { join, relative } from "node:path"; +import { describe, expect, test } from "bun:test"; + +import { + countScannedFiles, + evaluateGuard, + isAllowlisted, + isCoverageEnough, + isGuardPassing, + loadAllowlist, + loadGuardConfig, + parseAllowlistText, + parseGuardConfig, + parseTsPruneLine, + validateAllowlistEntry, + validateAllowlistOwnership, + validateAllowlistText, +} from "./check-dead-exports.js"; + +const repoRoot = join(import.meta.dir, ".."); + +// A probe dead export in one of the scoped files must fail the guard: the +// exact-name exemptions cover only the five deferred-cleanup flags, never +// the whole module. +describe("scoped exemptions", () => { + test("the real allowlist covers the named flags but not a sibling probe", () => { + const rules = loadAllowlist(); + expect( + isAllowlisted(rules, "src/auth/codex/usage.ts", "fetchCodexUsage"), + ).toBe(true); + expect( + isAllowlisted(rules, "src/auth/codex/usage.ts", "fetchCodexModels"), + ).toBe(true); + expect(isAllowlisted(rules, "src/auth/xai/usage.ts", "fetchXaiUsage")).toBe( + true, + ); + expect( + isAllowlisted( + rules, + "src/auth/codex/constants.ts", + "CODEX_REFRESH_SKEW_MS", + ), + ).toBe(true); + expect( + isAllowlisted( + rules, + "src/auth/codex/constants.ts", + "CODEX_HEADLESS_REFRESH_INTERVAL_MS", + ), + ).toBe(true); + expect( + isAllowlisted(rules, "src/auth/codex/usage.ts", "someNewDeadExport"), + ).toBe(false); + }); + + test("a probe dead export in a scoped file is a violation", () => { + const rules = parseAllowlistText( + "src/auth/codex/usage.ts: fetchCodexUsage\n", + ); + const outcome = evaluateGuard( + rules, + "src/auth/codex/usage.ts:114 - fetchCodexUsage\n" + + "src/auth/codex/usage.ts:200 - someNewDeadExport\n", + ); + expect(outcome.dead).toBe(2); + expect(outcome.violations).toEqual([ + "src/auth/codex/usage.ts: someNewDeadExport", + ]); + expect(outcome.unused).toEqual([]); + }); + + test("exports used only inside their own module do not count", () => { + const outcome = evaluateGuard( + [], + "src/inference-abort.ts:5 - InferenceAbortReason (used in module)\n", + ); + expect(outcome.dead).toBe(0); + expect(outcome.violations).toEqual([]); + }); + + test("parseTsPruneLine skips blank and unparseable lines", () => { + expect(parseTsPruneLine("")).toBeUndefined(); + expect(parseTsPruneLine("not a ts-prune line")).toBeUndefined(); + expect( + parseTsPruneLine("src/auth/xai/usage.ts:89 - fetchXaiUsage"), + ).toEqual({ file: "src/auth/xai/usage.ts", name: "fetchXaiUsage" }); + }); +}); + +// Allowlist entries that match no current ts-prune flag are stale: they fail +// the gate so the exemption is removed with the code it covered. Warn-only +// reporting let dead exemptions linger silently after the code was gone. +describe("stale allowlist entries", () => { + test("an entry matching nothing is reported as unused and fails the gate", () => { + const rules = parseAllowlistText( + "src/auth/xai/usage.ts: fetchXaiUsage\n" + + "src/gone.ts: vanishedExport\n", + ); + const outcome = evaluateGuard( + rules, + "src/auth/xai/usage.ts:89 - fetchXaiUsage\n", + ); + expect(outcome.violations).toEqual([]); + expect(outcome.unused).toEqual(["src/gone.ts: vanishedExport"]); + expect(isGuardPassing(outcome)).toBe(false); + }); + + test("a fully fresh allowlist passes the gate", () => { + const rules = parseAllowlistText( + "vendor/\nsrc/auth/xai/usage.ts: fetchXaiUsage\n", + ); + const outcome = evaluateGuard( + rules, + "vendor/intx-types/src/index.ts:3 - ErrorResponse\n" + + "src/auth/xai/usage.ts:89 - fetchXaiUsage\n", + ); + expect(outcome.violations).toEqual([]); + expect(outcome.unused).toEqual([]); + expect(isGuardPassing(outcome)).toBe(true); + }); + + test("a prefix entry counts as used when any flag falls under it", () => { + const rules = parseAllowlistText("vendor/\n"); + const outcome = evaluateGuard( + rules, + "vendor/intx-types/src/index.ts:3 - ErrorResponse\n", + ); + expect(outcome.violations).toEqual([]); + expect(outcome.unused).toEqual([]); + expect(isGuardPassing(outcome)).toBe(true); + }); + + test("violations fail the gate", () => { + const outcome = evaluateGuard([], "src/new.ts:1 - freshDeadExport\n"); + expect(outcome.violations).toEqual(["src/new.ts: freshDeadExport"]); + expect(isGuardPassing(outcome)).toBe(false); + }); +}); + +// Entry shapes the matcher would silently misinterpret must fail validation +// instead: a mistyped exact entry must not decay into a prefix that matches +// nothing, and a directory without its trailing slash must not pass as an +// imprecise prefix. +describe("allowlist entry shapes", () => { + test("valid entries pass", () => { + expect(validateAllowlistEntry("vendor/")).toBeUndefined(); + expect(validateAllowlistEntry("src/auth/codex/usage.ts")).toBeUndefined(); + expect( + validateAllowlistEntry("src/auth/codex/usage.ts: fetchCodexUsage"), + ).toBeUndefined(); + expect( + validateAllowlistEntry( + "tests/fixtures/plugins/implement-feature/src/index.ts", + ), + ).toBeUndefined(); + }); + + test("slash-less directory prefixes fail", () => { + expect(validateAllowlistEntry("vendor")).toBeDefined(); + expect(validateAllowlistEntry("src/auth")).toBeDefined(); + expect(validateAllowlistEntry("/")).toBeDefined(); + }); + + test("malformed exact entries fail instead of decaying into prefixes", () => { + expect( + validateAllowlistEntry("src/auth/codex/usage.ts: bad name!"), + ).toBeDefined(); + expect( + validateAllowlistEntry("src/auth/codex/usage.ts: 123abc"), + ).toBeDefined(); + expect(validateAllowlistEntry("src/auth/codex/usage.ts:")).toBeDefined(); + expect(validateAllowlistEntry("usage.ts: fetchCodexUsage")).toBeDefined(); + }); + + test("entries with whitespace fail", () => { + expect(validateAllowlistEntry("src/has space/x.ts")).toBeDefined(); + }); + + test("the checked-in allowlist passes shape validation", () => { + const text = readFileSync( + join(repoRoot, "scripts", "dead-export-allowlist.txt"), + "utf8", + ); + expect(validateAllowlistText(text)).toEqual([]); + }); +}); + +// This repo has no CODEOWNERS, so the documented review convention is that +// every entry block names its owning lane in the reason comment above it. +// The gate enforces the reason comments; human review enforces the lane. +describe("allowlist ownership", () => { + test("an entry under a reason comment passes", () => { + expect( + validateAllowlistOwnership("# Owner: usage-data lane\nsrc/a.ts: Thing\n"), + ).toEqual([]); + }); + + test("a reason block covers the contiguous entries below it", () => { + expect( + validateAllowlistOwnership( + "# Owner: usage-data lane\nsrc/a.ts: Thing\nsrc/b.ts: Other\n", + ), + ).toEqual([]); + }); + + test("an entry with no reason comment fails", () => { + expect(validateAllowlistOwnership("src/a.ts: Thing\n")).toEqual([ + "allowlist entry without a reason comment naming its owner: src/a.ts: Thing", + ]); + }); + + test("a new section after a blank line needs its own reason", () => { + expect( + validateAllowlistOwnership( + "# Owner: usage-data lane\nsrc/a.ts: Thing\n\nsrc/b.ts: Other\n", + ), + ).toEqual([ + "allowlist entry without a reason comment naming its owner: src/b.ts: Other", + ]); + }); + + test("a bare hash is not a reason", () => { + expect(validateAllowlistOwnership("#\nsrc/a.ts: Thing\n")).toEqual([ + "allowlist entry without a reason comment naming its owner: src/a.ts: Thing", + ]); + }); + + test("the checked-in allowlist names an owner for every entry", () => { + const text = readFileSync( + join(repoRoot, "scripts", "dead-export-allowlist.txt"), + "utf8", + ); + expect(validateAllowlistOwnership(text)).toEqual([]); + }); +}); + +// The scan invocation is pinned to scripts/dead-export-guard.json so it never +// depends on ts-prune's working-directory config discovery, and the gate +// fails closed when the scanned file count drops below the checked-in floor +// instead of green-lighting a scan that looked at less code. +describe("pinned scan invocation", () => { + test("the checked-in config pins the project and a positive floor", () => { + const config = loadGuardConfig(); + expect(config.tsconfig).toBe("tsconfig.json"); + expect(config.tsPruneArgs).toEqual(["-p", "tsconfig.json"]); + expect(config.minScannedFiles).toBeGreaterThan(0); + expect(existsSync(join(repoRoot, config.tsconfig))).toBe(true); + }); + + test("parseGuardConfig rejects an unpinned or empty invocation", () => { + const valid = { + tsconfig: "tsconfig.json", + tsPruneArgs: ["-p", "tsconfig.json"], + minScannedFiles: 1130, + }; + expect(parseGuardConfig(valid)).toEqual(valid); + expect(() => parseGuardConfig({ ...valid, tsPruneArgs: [] })).toThrow(); + expect(() => + parseGuardConfig({ ...valid, tsPruneArgs: ["--ignore", "x"] }), + ).toThrow(); + expect(() => + parseGuardConfig({ + ...valid, + tsPruneArgs: ["-p", "tsconfig.other.json"], + }), + ).toThrow(); + }); + + test("parseGuardConfig rejects extra narrowing flags on a pinned invocation", () => { + const valid = { + tsconfig: "tsconfig.json", + tsPruneArgs: ["-p", "tsconfig.json"], + minScannedFiles: 1130, + }; + expect(parseGuardConfig(valid)).toEqual(valid); + const narrowed = [ + ["-p", "tsconfig.json", "-i", "src/.*"], + ["-p", "tsconfig.json", "--ignore", "src/.*"], + ["-p", "tsconfig.json", "--error"], + ["--ignore", "src/.*", "-p", "tsconfig.json"], + ]; + for (const tsPruneArgs of narrowed) { + expect(() => parseGuardConfig({ ...valid, tsPruneArgs })).toThrow(); + } + }); + + test("parseGuardConfig rejects a missing floor", () => { + const valid = { + tsconfig: "tsconfig.json", + tsPruneArgs: ["-p", "tsconfig.json"], + minScannedFiles: 1130, + }; + for (const floor of [0, -5, 1.5, "1130", undefined]) { + expect(() => + parseGuardConfig({ ...valid, minScannedFiles: floor }), + ).toThrow(); + } + expect(() => parseGuardConfig(null)).toThrow(); + expect(() => parseGuardConfig([])).toThrow(); + }); +}); + +describe("scan coverage floor", () => { + test("counts below the floor fail, counts at or above pass", () => { + expect(isCoverageEnough(1129, 1130)).toBe(false); + expect(isCoverageEnough(1130, 1130)).toBe(true); + expect(isCoverageEnough(2000, 1130)).toBe(true); + }); + + test("the live program file count clears the checked-in floor", () => { + const config = loadGuardConfig(); + const scanned = countScannedFiles(repoRoot, config.tsconfig); + expect(scanned).toBeGreaterThanOrEqual(config.minScannedFiles); + }, 120_000); + + test("the checked-in floor stays tight to the live count", () => { + const config = loadGuardConfig(); + const scanned = countScannedFiles(repoRoot, config.tsconfig); + expect(scanned).toBeLessThan(config.minScannedFiles * 1.1); + }, 120_000); +}); + +// The unit tests above prove the rule engine flags a probe; this one proves +// the wired-up gate does. A temp probe export lands in the tsconfig-covered +// scripts/ tree, the real guard runs as a subprocess, and the run must exit +// nonzero naming the probe. The probe lives only for the test (never +// committed) so the keep-alive check cannot itself become a dead export. +describe("violation end to end", () => { + test("a temp dead export fails the guard, which names it", () => { + const probeFile = `dead-export-guard-probe-${process.pid}.ts`; + const probeName = `deadExportGuardProbe${process.pid}`; + const probePath = join(repoRoot, "scripts", probeFile); + writeFileSync(probePath, `export const ${probeName} = 1;\n`); + try { + const ran = spawnSync( + process.execPath, + ["scripts/check-dead-exports.ts"], + { cwd: repoRoot, encoding: "utf8" }, + ); + expect(ran.status).toBe(1); + expect(`${ran.stdout}\n${ran.stderr}`).toContain( + `${probeFile}: ${probeName}`, + ); + } finally { + rmSync(probePath, { force: true }); + } + }, 120_000); +}); + +// The purge deleted four fully-dead barrel files; a re-created barrel (or a +// new import of its path) would silently resurrect the surface the guard was +// built to shrink. Pin the paths, not the file text. +describe("deleted barrels stay deleted", () => { + const barrels = [ + "src/agent/directors/index.ts", + "src/auth/codex/index.ts", + "src/auth/xai/index.ts", + "src/web/index.ts", + ]; + const barrelDirs = barrels.map((barrel) => + barrel.slice(0, -"/index.ts".length), + ); + const barrelTails = [ + "agent/directors/index", + "auth/codex/index", + "auth/xai/index", + "web/index", + ]; + const roots = ["src", "tests", "evals", "scripts", "packages"]; + + test("the barrel files do not exist", () => { + for (const barrel of barrels) { + expect(existsSync(join(repoRoot, barrel))).toBe(false); + } + }); + + test("no source file imports the deleted barrel paths", () => { + const offenders: string[] = []; + const walk = (dir: string): void => { + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const absolute = join(dir, entry.name); + if (entry.isDirectory()) { + walk(absolute); + continue; + } + if (!entry.name.endsWith(".ts")) continue; + const rel = relative(repoRoot, absolute).split("/").join("/"); + const dirRel = rel.slice(0, -entry.name.length - 1); + const text = readFileSync(absolute, "utf8"); + const specifiers = [ + ...text.matchAll(/(?:from\s*["']|import\s*\(\s*["'])([^"']+)["']/g), + ].map((match) => (match[1] ?? "").replace(/\.(?:js|ts)$/, "")); + for (const spec of specifiers) { + const hitsBarrel = + barrelTails.some( + (tail) => spec === tail || spec.endsWith(`/${tail}`), + ) || + (spec === "./index" && barrelDirs.includes(dirRel)); + if (hitsBarrel) offenders.push(`${rel}: ${spec}`); + } + } + }; + for (const root of roots) walk(join(repoRoot, root)); + expect(offenders).toEqual([]); + }); +}); diff --git a/scripts/check-dead-exports.ts b/scripts/check-dead-exports.ts index 971fa1553..5d212fbc7 100644 --- a/scripts/check-dead-exports.ts +++ b/scripts/check-dead-exports.ts @@ -1,44 +1,219 @@ import { spawnSync } from "node:child_process"; -import { readFileSync } from "node:fs"; -import { dirname, join } from "node:path"; +import { existsSync, readFileSync, realpathSync } from "node:fs"; +import { dirname, join, sep } from "node:path"; import { fileURLToPath } from "node:url"; -// Dead-export guard (CL-6797): runs ts-prune over the project and fails when -// any export with no consumer falls outside scripts/dead-export-allowlist.txt. -// Exports used only inside their own module ("(used in module)") are live -// enough and do not count. New dead exports must be deleted, not allowlisted: -// the allowlist covers entry points, cross-lane ownership, plugin surfaces -// loaded by path, and ts-prune parser false positives only. +// Dead-export guard (CL-6797, hardened CL-7993): runs ts-prune over the project +// and fails when any export with no consumer falls outside +// scripts/dead-export-allowlist.txt. Exports used only inside their own module +// ("(used in module)") are live enough and do not count. New dead exports must +// be deleted, not allowlisted: the allowlist covers entry points, cross-lane +// ownership, plugin surfaces loaded by path, and ts-prune parser false +// positives only. +// +// Hardening: stale allowlist entries fail the gate instead of warning, every +// entry must pass shape validation and sit under a reason comment (the gate +// enforces the reason's presence; review enforces the owning lane — this repo +// has no CODEOWNERS, so the allowlist header documents the review convention), +// the ts-prune invocation is pinned to scripts/dead-export-guard.json, and the +// gate fails closed when the scanned file count drops below that config's +// floor. const here = dirname(fileURLToPath(import.meta.url)); const repoRoot = dirname(here); const allowlistPath = join(here, "dead-export-allowlist.txt"); +const guardConfigPath = join(here, "dead-export-guard.json"); const tsPruneBin = join(repoRoot, "node_modules", ".bin", "ts-prune"); +const tscBin = join(repoRoot, "node_modules", "typescript", "bin", "tsc"); -type AllowRule = - | { readonly kind: "prefix"; readonly prefix: string } - | { readonly kind: "exact"; readonly file: string; readonly name: string }; +export type AllowRule = + | { + readonly kind: "prefix"; + readonly prefix: string; + readonly source: string; + } + | { + readonly kind: "exact"; + readonly file: string; + readonly name: string; + readonly source: string; + }; + +export interface DeadExport { + readonly file: string; + readonly name: string; +} -function loadAllowlist(): AllowRule[] { +export interface GuardOutcome { + readonly dead: number; + readonly violations: string[]; + readonly unused: string[]; +} + +export interface GuardConfig { + readonly tsconfig: string; + readonly tsPruneArgs: readonly string[]; + readonly minScannedFiles: number; +} + +const exactEntryPattern = /^(.+?): ([A-Za-z_$][\w$]*)$/; + +export function parseAllowlistText(text: string): AllowRule[] { const rules: AllowRule[] = []; - for (const raw of readFileSync(allowlistPath, "utf8").split("\n")) { + for (const raw of text.split("\n")) { const line = raw.trim(); if (line === "" || line.startsWith("#")) continue; - const exact = line.match(/^(.+?): ([A-Za-z_$][\w$]*)$/); + const exact = line.match(exactEntryPattern); if (exact) { const file = exact[1]; const name = exact[2]; if (file !== undefined && name !== undefined) { - rules.push({ kind: "exact", file: file.trim(), name: name.trim() }); + rules.push({ + kind: "exact", + file: file.trim(), + name: name.trim(), + source: line, + }); } } else { - rules.push({ kind: "prefix", prefix: line }); + rules.push({ kind: "prefix", prefix: line, source: line }); } } return rules; } -function isAllowlisted( +// Rejects a single allowlist line that the matcher would silently +// misinterpret: a mistyped exact entry ("file: bad name!") must not decay into +// a prefix that matches nothing, and a directory without a trailing slash +// ("vendor") must not pass as an imprecise prefix. Only `dir/` prefixes and +// repo-relative `.ts` paths (bare or `file: Name`) are valid. +export function validateAllowlistEntry(line: string): string | undefined { + if (line.includes(":")) { + const exact = line.match(exactEntryPattern); + if (exact === null) { + return `malformed allowlist entry (want "path/to/file.ts: ExportName"): ${line}`; + } + const file = exact[1] ?? ""; + if (/\s/.test(file) || !file.includes("/") || !file.endsWith(".ts")) { + return `allowlist entry file must be a repo-relative .ts path: ${line}`; + } + return undefined; + } + if (/\s/.test(line)) { + return `allowlist entry contains whitespace: ${line}`; + } + if (line.endsWith("/")) { + if (line.length < 2) { + return `malformed allowlist entry: ${line}`; + } + return undefined; + } + if (!line.includes("/") || !line.endsWith(".ts")) { + return `directory prefixes must end in "/" and files must end in ".ts": ${line}`; + } + return undefined; +} + +export function validateAllowlistText(text: string): string[] { + const problems: string[] = []; + for (const raw of text.split("\n")) { + const line = raw.trim(); + if (line === "" || line.startsWith("#")) continue; + const problem = validateAllowlistEntry(line); + if (problem !== undefined) problems.push(problem); + } + return problems; +} + +// Every entry must sit under a reason comment in the same blank-line section +// and above the entry. The gate enforces the reason's presence; human review +// enforces that it names the owning lane. A section of entries with no reason +// above it fails, so exemptions cannot land without a reason on record. +export function validateAllowlistOwnership(text: string): string[] { + const problems: string[] = []; + let reasoned = false; + for (const raw of text.split("\n")) { + const line = raw.trim(); + if (line === "") { + reasoned = false; + continue; + } + if (line.startsWith("#")) { + if (line.length > 1) reasoned = true; + continue; + } + if (!reasoned) { + problems.push( + `allowlist entry without a reason comment naming its owner: ${line}`, + ); + } + } + return problems; +} + +export function loadAllowlist(): AllowRule[] { + return parseAllowlistText(readFileSync(allowlistPath, "utf8")); +} + +// Parses and validates the pinned scan config. The invocation must be exactly +// "-p " and nothing else, so narrowing flags (e.g. "-i"/"--ignore") +// cannot shrink the scan while the tsc --listFilesOnly file count stays flat. +// The floor must be a positive integer the gate fails closed against. +export function parseGuardConfig(raw: unknown): GuardConfig { + if (typeof raw !== "object" || raw === null || Array.isArray(raw)) { + throw new Error("dead-export guard config must be a JSON object"); + } + const config = raw as Record; + const tsconfig = config["tsconfig"]; + if (typeof tsconfig !== "string" || tsconfig === "") { + throw new Error('dead-export guard config needs a "tsconfig" path string'); + } + const tsPruneArgs = config["tsPruneArgs"]; + if ( + !Array.isArray(tsPruneArgs) || + tsPruneArgs.length === 0 || + tsPruneArgs.some((arg) => typeof arg !== "string") + ) { + throw new Error( + 'dead-export guard config needs a non-empty "tsPruneArgs" string array', + ); + } + const args = tsPruneArgs as string[]; + if (args.length !== 2 || args[0] !== "-p" || args[1] !== tsconfig) { + throw new Error( + 'dead-export guard config "tsPruneArgs" must be exactly ["-p", ""] with no extra flags', + ); + } + const minScannedFiles = config["minScannedFiles"]; + if ( + typeof minScannedFiles !== "number" || + !Number.isInteger(minScannedFiles) || + minScannedFiles <= 0 + ) { + throw new Error( + 'dead-export guard config needs a positive integer "minScannedFiles"', + ); + } + return { tsconfig, tsPruneArgs: [...args], minScannedFiles }; +} + +export function loadGuardConfig(): GuardConfig { + let raw: unknown; + try { + raw = JSON.parse(readFileSync(guardConfigPath, "utf8")); + } catch (err) { + throw new Error( + `cannot read ${guardConfigPath}: ${(err as Error).message}`, + ); + } + const config = parseGuardConfig(raw); + if (!existsSync(join(repoRoot, config.tsconfig))) { + throw new Error(`tsconfig not found: ${config.tsconfig}`); + } + return config; +} + +export function isAllowlisted( rules: AllowRule[], file: string, name: string, @@ -50,35 +225,152 @@ function isAllowlisted( ); } -function main(): void { - const rules = loadAllowlist(); - const pruned = spawnSync(tsPruneBin, [], { cwd: repoRoot, encoding: "utf8" }); - if (pruned.status !== 0) { - console.error(`ts-prune failed:\n${pruned.stderr || pruned.stdout}`); - process.exit(1); - } +// Parses one ts-prune output line. Returns undefined for blank lines, +// unparseable lines, and exports used only inside their own module. +export function parseTsPruneLine(raw: string): DeadExport | undefined { + const match = raw.match(/^(.*?):\d+ - (\S+?)(\s+\(used in module\))?$/); + if (!match) return undefined; + if (match[3] !== undefined) return undefined; + const file = match[1]; + const name = match[2]; + if (file === undefined || name === undefined) return undefined; + return { file, name }; +} + +export function evaluateGuard( + rules: AllowRule[], + tsPruneStdout: string, +): GuardOutcome { + const used = new Set(); const violations: string[] = []; let dead = 0; - for (const raw of String(pruned.stdout).split("\n")) { - const match = raw.match(/^(.*?):\d+ - (\S+?)(\s+\(used in module\))?$/); - if (!match) continue; - if (match[3] !== undefined) continue; - const file = match[1]; - const name = match[2]; - if (file === undefined || name === undefined) continue; + for (const raw of tsPruneStdout.split("\n")) { + const parsed = parseTsPruneLine(raw); + if (parsed === undefined) continue; dead += 1; - if (!isAllowlisted(rules, file, name)) violations.push(`${file}: ${name}`); + const index = rules.findIndex((rule) => + rule.kind === "prefix" + ? parsed.file.startsWith(rule.prefix) + : rule.file === parsed.file && rule.name === parsed.name, + ); + if (index === -1) { + violations.push(`${parsed.file}: ${parsed.name}`); + } else { + used.add(index); + } + } + const unused = rules + .filter((_, index) => !used.has(index)) + .map((r) => r.source); + return { dead, violations, unused }; +} + +// The gate passes only when nothing new died and no exemption is stale. +// Stale entries fail (they used to warn) so dead exemptions cannot linger +// after the code they cover is gone. +export function isGuardPassing(outcome: GuardOutcome): boolean { + return outcome.violations.length === 0 && outcome.unused.length === 0; +} + +// Counts the TypeScript files the pinned tsconfig pulls into its program via +// tsc --listFilesOnly: the same project ts-prune analyzes. A narrowed +// tsconfig (or a moved scan root) shrinks this count, and the gate fails +// closed against the checked-in floor instead of green-lighting a scan that +// looked at less code. +export function countScannedFiles( + repoRootDir: string, + tsconfigPath: string, +): number { + const ran = spawnSync(tscBin, ["-p", tsconfigPath, "--listFilesOnly"], { + cwd: repoRootDir, + encoding: "utf8", + }); + if (ran.error !== undefined) { + throw new Error(`tsc --listFilesOnly failed to start: ${ran.error}`); + } + const roots = [repoRootDir, realpathSync(repoRootDir)]; + let count = 0; + for (const raw of String(ran.stdout).split("\n")) { + const line = raw.trim(); + if (line === "") continue; + if (!line.endsWith(".ts") && !line.endsWith(".tsx")) continue; + if (line.includes(`${sep}node_modules${sep}`)) continue; + if (!roots.some((root) => line.startsWith(root + sep))) continue; + count += 1; + } + return count; +} + +export function isCoverageEnough(scannedFiles: number, floor: number): boolean { + return scannedFiles >= floor; +} + +function fail(message: string): never { + console.error(message); + process.exit(1); +} + +function main(): void { + let config: GuardConfig; + try { + config = loadGuardConfig(); + } catch (err) { + fail(`dead-export guard: invalid guard config: ${(err as Error).message}`); + } + const allowlistText = readFileSync(allowlistPath, "utf8"); + const allowlistProblems = [ + ...validateAllowlistText(allowlistText), + ...validateAllowlistOwnership(allowlistText), + ]; + if (allowlistProblems.length > 0) { + fail( + "Invalid allowlist entries (fix the shape or remove them):\n" + + allowlistProblems.map((problem) => ` ${problem}`).join("\n"), + ); + } + const rules = parseAllowlistText(allowlistText); + const pruned = spawnSync(tsPruneBin, [...config.tsPruneArgs], { + cwd: repoRoot, + encoding: "utf8", + }); + if (pruned.status !== 0) { + fail(`ts-prune failed:\n${pruned.stderr || pruned.stdout}`); + } + const outcome = evaluateGuard(rules, String(pruned.stdout)); + let scannedFiles: number; + try { + scannedFiles = countScannedFiles(repoRoot, config.tsconfig); + } catch (err) { + fail( + `dead-export guard: could not count scanned files: ${(err as Error).message}`, + ); } console.log( - `dead-export guard: ${dead} consumer-less exports, ${dead - violations.length} allowlisted, ${violations.length} violations`, + `dead-export guard: ${outcome.dead} consumer-less exports, ${outcome.dead - outcome.violations.length} allowlisted, ${outcome.violations.length} violations, ${scannedFiles} scanned files (floor ${config.minScannedFiles})`, ); - if (violations.length > 0) { + if (!isCoverageEnough(scannedFiles, config.minScannedFiles)) { + fail( + `dead-export guard: scanned file count ${scannedFiles} is below the floor ${config.minScannedFiles} ` + + "(the scan narrowed; fix the tsconfig or update scripts/dead-export-guard.json)", + ); + } + if (outcome.unused.length > 0) { + console.error( + "Stale allowlist entries matching no dead export (remove them):\n" + + outcome.unused.map((entry) => ` ${entry}`).join("\n"), + ); + } + if (outcome.violations.length > 0) { console.error( "New dead exports (delete them or justify an allowlist entry):\n" + - violations.map((v) => ` ${v}`).join("\n"), + outcome.violations.map((v) => ` ${v}`).join("\n"), ); + } + if (!isGuardPassing(outcome)) { process.exit(1); } } -main(); +if (import.meta.main) { + main(); +} diff --git a/scripts/dead-export-allowlist.txt b/scripts/dead-export-allowlist.txt index d28067ad7..2041c4f28 100644 --- a/scripts/dead-export-allowlist.txt +++ b/scripts/dead-export-allowlist.txt @@ -3,7 +3,14 @@ # Format: `#` lines are reasons and attach to the entries below them. An entry # is either a path prefix ending in `/` (matches a directory subtree), a full # file path (matches every export in that file), or `file: name` (matches one -# export). Every entry must sit under a reason comment. +# export). Anything else (a directory without its trailing `/`, a `file:` +# line that is not `path/to/file.ts: ExportName`) fails the gate. +# +# Review convention: this repo has no CODEOWNERS, so every entry block should +# name its owning lane in the reason comment above it, and changes to an +# entry need that lane's review. The gate rejects entries with no reason +# comment above them (presence only), and stale entries (matching no current ts-prune flag) +# fail the gate: remove the entry with the code it covered. # # Everything NOT listed here must have zero ts-prune flags: any export with no # consumer gets deleted instead of allowlisted. @@ -13,11 +20,17 @@ # vendor workspaces on the next sync. Prune upstream, not here. vendor/ -# CL-6815 lane owns the test-only consumer modules (Codex/xAI usage data -# paths). That lane decides their fate; this lane must not delete them. -src/auth/codex/usage.ts -src/auth/xai/usage.ts -src/auth/codex/constants.ts +# Deferred Codex/xAI usage-data cleanup owned by the CL-6815 lane: these +# modules are fully dead deferred code with no live importers (not test-only +# helpers), and that lane decides their fate. Scoped to the exact ts-prune +# flags so a new dead export in these files fails the guard instead of +# hiding under a whole-file exemption. Remove each entry with the export it +# names when the owning cleanup lands. +src/auth/codex/usage.ts: fetchCodexUsage +src/auth/codex/usage.ts: fetchCodexModels +src/auth/xai/usage.ts: fetchXaiUsage +src/auth/codex/constants.ts: CODEX_REFRESH_SKEW_MS +src/auth/codex/constants.ts: CODEX_HEADLESS_REFRESH_INTERVAL_MS # Plugin fixture entry point. Loaded by file path from the fixture's plugin # manifest by the plugin-registration tests, so it has no static importers. diff --git a/scripts/dead-export-guard.json b/scripts/dead-export-guard.json new file mode 100644 index 000000000..9b2bf9e8f --- /dev/null +++ b/scripts/dead-export-guard.json @@ -0,0 +1,5 @@ +{ + "tsconfig": "tsconfig.json", + "tsPruneArgs": ["-p", "tsconfig.json"], + "minScannedFiles": 1130 +}