diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 97c71a4dcf..52c5867032 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -174,6 +174,11 @@ jobs: - name: Install run: pnpm install --frozen-lockfile + # Must stay `pnpm run typecheck`, not a bare `tsc --noEmit`. The script is + # a wrapper (scripts/typecheck.mjs) that sets the V8 heap cap AND + # classifies the outcome: a `tsc` that dies of heap exhaustion emits zero + # diagnostics, so a raw invocation produces an empty log that reads as a + # clean pass to anyone scanning it. The wrapper says CRASHED instead. - name: Typecheck run: pnpm run typecheck diff --git a/.husky/pre-push b/.husky/pre-push index abe05a7371..bc9adb10ef 100644 --- a/.husky/pre-push +++ b/.husky/pre-push @@ -12,6 +12,18 @@ fi echo "Running typecheck for all files" +# The exit code has to be read and re-raised. `sh` without `set -e` runs the next +# line regardless, so an unconditional "Typecheck successful" echo became the +# hook's own exit status — a failing typecheck printed success and the push went +# through. Verified: `sh -c 'false; echo ok'` exits 0. npm run typecheck +rc=$? + +if [ "$rc" -ne 0 ]; then + echo "Typecheck FAILED (exit $rc) — push blocked." + echo "If the output above has no type errors in it, the typechecker CRASHED;" + echo "scripts/typecheck.mjs says which. A crash is not a passing typecheck." + exit "$rc" +fi echo "Typecheck successful" diff --git a/package.json b/package.json index f18f4624f5..28cc6227e4 100644 --- a/package.json +++ b/package.json @@ -65,7 +65,7 @@ "size": "node scripts/bundle-budget.mjs", "deploy": "pnpm run build && pnpm run db:deploy", "postinstall": "pnpm run db:generate", - "typecheck": "cross-env NODE_OPTIONS=\"--max_old_space_size=8192\" tsc --noEmit", + "typecheck": "node scripts/typecheck.mjs", "lint": "eslint src/ --cache --cache-strategy metadata", "lint:packages": "eslint packages --ext .ts", "eslint": "cross-env TIMING=1 eslint src/ --quiet --cache --cache-strategy metadata", diff --git a/scripts/__tests__/typecheck.test.ts b/scripts/__tests__/typecheck.test.ts new file mode 100644 index 0000000000..f7b3a6bbad --- /dev/null +++ b/scripts/__tests__/typecheck.test.ts @@ -0,0 +1,115 @@ +import { spawnSync } from 'node:child_process'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; + +/** + * `scripts/typecheck.mjs` exists to stop a CRASHED typecheck from reading as a + * clean one. The crash it was built for — V8 aborting on heap exhaustion — emits + * zero diagnostics and writes its explanation to stderr, so a caller that greps + * for type errors, or that captures only stdout, sees an empty log. + * + * These cases drive the wrapper with stub "typecheckers" that exit the way each + * real outcome does. That keeps the classifier under test at sub-second cost; a + * real repo-wide `tsc` run is minutes, so nobody would run it per-case. + */ + +const WRAPPER = path.resolve(__dirname, '../typecheck.mjs'); + +let dir: string; +const stub = (name: string, body: string) => { + const file = path.join(dir, `${name}.mjs`); + writeFileSync(file, body); + return file; +}; + +// The wrapper always appends --noEmit; stubs ignore argv. +const run = (tscPath: string) => + spawnSync(process.execPath, [WRAPPER], { + encoding: 'utf8', + env: { ...process.env, TYPECHECK_TSC_PATH: tscPath, GITHUB_ACTIONS: '' }, + }); + +const CRASH_MARKER = 'TYPECHECK CRASHED'; +const OK_MARKER = 'typecheck: OK'; +// Built by concatenation so this file's own source cannot be mistaken for a +// diagnostic by the same greps the wrapper is defending against. +const DIAGNOSTIC_MARKER = `error${' '}TS`; +const DIAGNOSTIC_LINE = `src/foo.ts(1,1): ${DIAGNOSTIC_MARKER}2322: Type 'string' is not assignable to type 'number'.`; + +beforeAll(() => { + dir = mkdtempSync(path.join(tmpdir(), 'typecheck-guard-')); +}); +afterAll(() => { + rmSync(dir, { recursive: true, force: true }); +}); + +describe('typecheck wrapper outcome classification', () => { + it('reports a clean run explicitly rather than by silence', () => { + const res = run(stub('clean', 'process.exit(0);\n')); + + expect(res.status).toBe(0); + // The success line is the point: a clean `tsc` prints nothing, which is + // byte-identical to a crash whose stderr was dropped. + expect(res.stdout).toContain(OK_MARKER); + expect(res.stdout).not.toContain(CRASH_MARKER); + }); + + it('passes ordinary type errors through untouched', () => { + const res = run( + stub('errors', `console.log(${JSON.stringify(DIAGNOSTIC_LINE)});\nprocess.exit(2);\n`) + ); + + expect(res.status).toBe(2); + expect(res.stdout).toContain(DIAGNOSTIC_MARKER); + // A real type error is not a crash, and must not be dressed up as one. + expect(res.stdout).not.toContain(CRASH_MARKER); + expect(res.stdout).not.toContain(OK_MARKER); + }); + + it('names heap exhaustion when V8 aborts with no diagnostics', () => { + const res = run( + stub( + 'oom', + [ + "console.error('<--- Last few GCs --->');", + "console.error('FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory');", + 'process.abort();', + ].join('\n') + ) + ); + + expect(res.status).not.toBe(0); + // Loud on stdout, because the log that hid this originally was stdout-only. + expect(res.stdout).toContain(CRASH_MARKER); + expect(res.stdout).not.toContain(OK_MARKER); + expect(res.stderr).toContain('ran out of old-space heap'); + // And the wrapper's own crash report must not contain the marker callers + // grep for, or the report itself reads as a type error. Checked on BOTH + // streams: the report is written to stderr, so a stdout-only assertion here + // passes with the marker reintroduced (it did, until this line was added). + expect(res.stdout).not.toContain(DIAGNOSTIC_MARKER); + expect(res.stderr).not.toContain(DIAGNOSTIC_MARKER); + }); + + it('distinguishes an outside kill from V8 running out of heap', () => { + const res = run(stub('sigkill', "process.kill(process.pid, 'SIGKILL');\n")); + + expect(res.status).not.toBe(0); + expect(res.stdout).toContain(CRASH_MARKER); + // Different cause, different fix: more RAM / a LOWER cap, not a higher one. + expect(res.stderr).toContain('killed from outside'); + expect(res.stderr).not.toContain('ran out of old-space heap'); + }); + + it('refuses to call it a pass when tsc exits 0 while printing diagnostics', () => { + const res = run( + stub('contradiction', `console.log(${JSON.stringify(DIAGNOSTIC_LINE)});\nprocess.exit(0);\n`) + ); + + expect(res.status).not.toBe(0); + expect(res.stdout).toContain(CRASH_MARKER); + expect(res.stdout).not.toContain(OK_MARKER); + }); +}); diff --git a/scripts/typecheck.mjs b/scripts/typecheck.mjs new file mode 100644 index 0000000000..6add5bebfe --- /dev/null +++ b/scripts/typecheck.mjs @@ -0,0 +1,240 @@ +#!/usr/bin/env node +/** + * Runs the full-repo `tsc --noEmit` and classifies the OUTCOME, rather than + * leaving that to whoever reads the log. + * + * Why this exists + * --------------- + * `tsc` on this repo needs a raised V8 old-space (the default cap is far below + * what the program graph costs). When the cap is too low, V8 aborts: + * + * FATAL ERROR: Ineffective mark-compacts near heap limit + * Allocation failed - JavaScript heap out of memory + * + * The abort happens PART WAY THROUGH checking, so the process emits **zero** + * `error TS` lines and dies. Through `pnpm run`, that surfaces as + * `ELIFECYCLE Command failed with exit code 1` — a non-zero exit with a log + * that contains no type errors at all. + * + * That is byte-for-byte indistinguishable from a clean run to anything that + * judges the typecheck by its OUTPUT ("no `error TS` lines => clean"), which is + * what humans and scripted checks usually do. Measured on this repo: with a + * deliberate `const x: number = 'nope'` in `src/`, a run capped at 4096 MB + * reported 0 errors and hid it completely, while the same tree at 8192 MB + * reported it. Raising the cap only moves that cliff — the codebase grows into + * it again — so the number alone is not the fix. This wrapper makes the crash + * LOUD and unmistakably distinct from both "clean" and "you have type errors". + * + * Classification + * -------------- + * exit 0, no `error TS` -> clean. + * exit != 0, has `error TS` -> ordinary type errors; exit code passed through. + * exit != 0, no `error TS` -> CRASH. Never report this as a typecheck result. + * Names heap exhaustion / kernel OOM-kill / + * unknown-crash separately, since the fixes differ. + * exit 0, has `error TS` -> defensive: tsc contradicting itself, treated as + * a crash rather than trusted. + * + * Heap size + * --------- + * Measured cold (no .tsbuildinfo) on a clean checkout, with a deliberate type + * error in `src/` as a visibility control: + * + * node 24.18.1 4096 MB -> OOM, 0 diagnostics 4608 MB -> OOM, 0 diagnostics + * 5120 MB -> pass, error found 8192 MB -> pass, error found + * node 22.22.2 6144 MB -> pass, error found 8192 MB -> pass, error found + * + * So the cliff sits between 4608 and 5120 MB, and the 8192 default carries ~1.6x + * headroom over it. It is deliberately NOT raised further: the CI runner has + * 16 GB, and a cap approaching that trades a self-describing V8 abort for a + * kernel OOM-kill, which says less. Override per-run with TYPECHECK_HEAP_MB. + * When the codebase does grow into 8192, the crash report below is what says so + * — out loud — instead of a silently empty log. + */ + +import { spawn } from 'node:child_process'; +import { createRequire } from 'node:module'; + +const require = createRequire(import.meta.url); + +const DEFAULT_HEAP_MB = 8192; +const heapMb = Number(process.env.TYPECHECK_HEAP_MB || DEFAULT_HEAP_MB); +if (!Number.isFinite(heapMb) || heapMb <= 0) { + console.error( + `typecheck: TYPECHECK_HEAP_MB must be a positive number, got "${process.env.TYPECHECK_HEAP_MB}"` + ); + process.exit(2); +} + +// TYPECHECK_TSC_PATH is a test seam: it lets the suite drive this classifier +// with a stub that exits the way a crashed / clean / erroring tsc does, without +// paying a multi-minute real typecheck per case. Not meant for normal use. +let tscPath = process.env.TYPECHECK_TSC_PATH; +if (!tscPath) { + try { + tscPath = require.resolve('typescript/lib/tsc.js'); + } catch { + console.error('typecheck: cannot resolve typescript/lib/tsc.js — run `pnpm install` first.'); + process.exit(2); + } +} + +const tscArgs = process.argv.slice(2); +const startedAt = Date.now(); +const child = spawn( + process.execPath, + [`--max-old-space-size=${heapMb}`, tscPath, '--noEmit', ...tscArgs], + { + // NODE_OPTIONS is dropped on purpose: the heap cap is passed as an argv flag + // so an inherited NODE_OPTIONS cannot silently override it, and so the value + // reported below is the value actually in force. + env: { ...process.env, NODE_OPTIONS: '' }, + stdio: ['inherit', 'pipe', 'pipe'], + } +); + +// Counted while streaming rather than buffered: a badly broken tree can emit +// tens of thousands of diagnostics, and this wrapper must not become the thing +// that runs out of memory. +let errorTsLines = 0; +let sawHeapOom = false; +const tail = []; +const TAIL_LINES = 40; + +const OOM_SIGNATURES = [ + 'JavaScript heap out of memory', + 'Ineffective mark-compacts near heap limit', + 'FATAL ERROR', +]; + +function onLine(line) { + // `tsc` diagnostics are `path(line,col): error TS####: message`. `--pretty` + // is not used by this script, so the plain form is what arrives. + if (line.includes('error TS')) errorTsLines++; + if (OOM_SIGNATURES.some((sig) => line.includes(sig))) sawHeapOom = true; + tail.push(line); + if (tail.length > TAIL_LINES) tail.shift(); +} + +function wire(stream, sink) { + let residual = ''; + stream.on('data', (chunk) => { + sink.write(chunk); + // Split on the residual + chunk so a signature straddling a chunk boundary + // is still seen whole. + const text = residual + chunk.toString('utf8'); + const lines = text.split('\n'); + residual = lines.pop() ?? ''; + for (const line of lines) onLine(line); + }); + stream.on('end', () => { + if (residual) onLine(residual); + }); +} + +wire(child.stdout, process.stdout); +wire(child.stderr, process.stderr); + +function annotate(message) { + // Surfaces in the checks UI when running under Actions; harmless elsewhere. + if (process.env.GITHUB_ACTIONS === 'true') console.log(`::error::${message}`); +} + +function reportCrash({ code, signal, reason, remedy }) { + const how = signal ? `killed by signal ${signal}` : `exit code ${code}`; + // The detail goes to stderr, but a one-line verdict also goes to STDOUT. + // The failure this whole wrapper exists to catch was found by a caller that + // redirected stdout and dropped stderr: V8's abort is written to stderr, so + // that caller saw an empty log and a bare non-zero exit. A crash has to be + // visible in whichever stream the reader happens to be capturing. + console.log(`TYPECHECK CRASHED (${how}): ${reason}. NOT a clean typecheck — see stderr.`); + console.error(''); + console.error('================================================================'); + console.error(' TYPECHECK CRASHED — THIS IS NOT A TYPECHECK RESULT'); + console.error('================================================================'); + // Nothing this wrapper prints may contain the literal diagnostic marker that + // callers grep for — otherwise a crash report reads as a type error to them. + console.error(` tsc ${how} after emitting 0 diagnostics.`); + console.error(` Cause: ${reason}`); + console.error(''); + console.error(' The check did not finish, so it says NOTHING about whether the'); + console.error(' code typechecks. Do not read the absence of errors as a pass.'); + console.error(''); + console.error(` Fix: ${remedy}`); + console.error(` Heap cap in force: --max-old-space-size=${heapMb} (MB).`); + console.error(' Override with TYPECHECK_HEAP_MB= pnpm run typecheck'); + if (tail.length) { + console.error(''); + console.error(` Last ${tail.length} line(s) of tsc output:`); + for (const line of tail) console.error(` ${line}`); + } + console.error('================================================================'); + annotate(`Typecheck crashed (${how}, 0 diagnostics emitted): ${reason}`); +} + +child.on('error', (err) => { + console.error(`typecheck: failed to start tsc: ${err.message}`); + process.exit(2); +}); + +child.on('close', (code, signal) => { + const crashed = signal !== null || code !== 0; + + if (!crashed) { + if (errorTsLines > 0) { + // tsc printed diagnostics and still claimed success. Whatever that is, it + // is not a verdict worth trusting. + reportCrash({ + code, + signal, + reason: `tsc exited 0 but printed ${errorTsLines} diagnostic line(s) — contradictory result`, + remedy: 'investigate the tsc invocation; do not treat this run as a pass.', + }); + process.exit(1); + } + // A clean `tsc --noEmit` prints NOTHING, so an empty log is exactly what a + // crash-with-stderr-dropped looks like too. Stating success explicitly means + // "the log has no errors in it" is no longer a thing anyone has to infer. + console.log( + `typecheck: OK — 0 type errors in ${((Date.now() - startedAt) / 1000).toFixed(0)}s ` + + `(heap cap ${heapMb} MB).` + ); + process.exit(0); + } + + if (errorTsLines > 0) { + // The ordinary, useful failure: real type errors. Pass the verdict through + // untouched so the existing developer experience is unchanged. + console.error(`\ntypecheck: ${errorTsLines} type error(s).`); + process.exit(code === 0 || code === null ? 1 : code); + } + + if (sawHeapOom) { + reportCrash({ + code, + signal, + reason: `V8 ran out of old-space heap at ${heapMb} MB`, + remedy: + 'raise the cap — TYPECHECK_HEAP_MB= to confirm, then raise ' + + 'DEFAULT_HEAP_MB in this script (and check the CI runner has the RAM for it).', + }); + } else if (signal === 'SIGKILL' || code === 137) { + reportCrash({ + code, + signal, + reason: + 'the process was killed from outside (out of system memory, or a job/container limit)', + remedy: + 'give the machine or runner more RAM, or LOWER the heap cap so V8 stays ' + + 'inside it — a cap above available RAM turns into an unhelpful kill.', + }); + } else { + reportCrash({ + code, + signal, + reason: 'tsc terminated abnormally without producing any diagnostics', + remedy: 'read the tsc output above; this is a tsc/tooling failure, not a code failure.', + }); + } + process.exit(code === 0 || code === null ? 1 : code); +}); diff --git a/vitest.config.mts b/vitest.config.mts index 3307e6836d..9ebc1bf312 100644 --- a/vitest.config.mts +++ b/vitest.config.mts @@ -68,7 +68,11 @@ export default defineConfig({ name: 'unit', globals: true, environment: 'node', - include: ['src/**/*.test.ts'], + // `scripts/` is included so the typecheck wrapper's outcome + // classifier (scripts/typecheck.mjs) is covered — it is the thing that + // decides whether a run counts as a pass, so it needs a suite of its + // own rather than a one-off manual check. + include: ['src/**/*.test.ts', 'scripts/**/*.test.ts'], exclude: ['node_modules', 'tests/**/*'], // Exclude Playwright tests setupFiles: ['src/__tests__/setup.ts'], // Several unit tests cold-`await import(...)` a large Next API-page / service