diff --git a/AGENTS.md b/AGENTS.md index 80eebc6b87..c95b6f1f0d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -34,7 +34,23 @@ that graduates deletes its ledger entry in the same PR. `typecheck` script advertises — a green gate over source nothing read, which is the #4311 defect itself. The ratchet's `TESTS_COVERED` invariant fails on any new exclusion; the packages that already had one carry a measured `TEST_DEBT` entry and graduate by -dropping the exclusion. +dropping the exclusion — or, when the build config must keep the exclusion (ci.yml gates +that no test file reaches the published artifact), by adding a **sibling +`tsconfig.test.json` and naming it in the `typecheck` script**, which is what +`packages/spec` does since #5286. The sibling may carry its own *module* semantics to +match how vitest executes the files (`module: esnext`, `moduleResolution: bundler`) — +never its own *strictness*: `strict` and friends are inherited, untouched. + +**A `@ts-expect-error` in a file no tsc program compiles is a phantom check** — the +`PINS_CHECKED` invariant of the same ratchet, repo-wide. `@ts-expect-error` is the +"tsc is the best sweeper" channel the spec-property-retirement playbook leans on: the +directive is meant to go red the day a removed key comes back. Outside a program it +evaluates never, and *deleting the directive leaves every gate just as green* — which is +how spec's 17 retirement pins across 5 files were found (#5286). Before writing one, +check the file is compiled. `packages/spec` additionally holds its test-layer residue in +a per-file, exactly-measured, shrink-only ledger (`packages/spec/test-typecheck-debt.json`, +`pnpm --filter @objectstack/spec gen:test-typecheck-debt`): a file not listed there may +have no type errors at all. One trap worth knowing before you read any of these counts: under `moduleResolution: NodeNext` a relative import missing its `.js` extension does not resolve, every symbol it diff --git a/packages/spec/package.json b/packages/spec/package.json index 2eed9e5cfd..4946eb5482 100644 --- a/packages/spec/package.json +++ b/packages/spec/package.json @@ -217,7 +217,9 @@ "check:react-blocks": "tsx scripts/build-react-blocks-contract.ts --check", "check:react-declaration-parity": "tsx scripts/check-react-blocks-declaration-parity.ts", "check:skill-examples": "tsx scripts/check-skill-examples.ts", - "typecheck": "tsc --noEmit" + "check:test-typecheck": "tsx scripts/check-test-typecheck.mts --self-test && tsx scripts/check-test-typecheck.mts --project tsconfig.test.json", + "gen:test-typecheck-debt": "tsx scripts/check-test-typecheck.mts --update --project tsconfig.test.json", + "typecheck": "tsc --noEmit && pnpm check:test-typecheck" }, "keywords": [ "objectstack", diff --git a/packages/spec/scripts/check-generated-ledger.test.ts b/packages/spec/scripts/check-generated-ledger.test.ts new file mode 100644 index 0000000000..482c472d21 --- /dev/null +++ b/packages/spec/scripts/check-generated-ledger.test.ts @@ -0,0 +1,81 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// Pins the `check:generated` LEDGER against package.json — the reconciliation +// that decides whether every `check:`/`gen:` script in this package is actually +// covered by a gate, or has quietly dropped out of coverage. +// +// WHY IT IS WORTH A TEST AND NOT JUST A CI STEP. The reconciliation has been +// dormant or unsatisfied three times now, and each time the cost was a CI lap +// rather than a local one: +// +// • #4177 and #4232 landed unclassified scripts while nothing in CI ran the +// reconciliation at all, so `main` carried a wrapper that exited red before +// running a single gate. +// • #4291 fixed that by wiring `--reconcile-only` into lint.yml's unfiltered +// required job — which is exactly where #5286's own first push then died, +// because the two scripts it added (`check:test-typecheck`, +// `gen:test-typecheck-debt`) were in neither ledger bucket. `tsc` passed; +// the step after it did not. +// +// So the negative direction of this reconciliation is not hypothetical — it is +// the reason this file exists, observed in production twice. What was missing +// was a signal BEFORE the push: `pnpm --filter @objectstack/spec test` did not +// read the ledger, so a script added in one file and unclassified in another was +// invisible until a runner said so ten minutes later. This closes that gap. +// +// It runs the real script in place: `--reconcile-only` reads package.json and +// the ledger arrays and exits — no gates, no build, no writes, sub-second — so +// there is nothing to sandbox and no way for it to differ from what CI runs. + +import { describe, it, expect } from 'vitest'; +import { spawnSync } from 'node:child_process'; +import { createRequire } from 'node:module'; +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const HERE = path.dirname(fileURLToPath(import.meta.url)); +const SPEC = path.resolve(HERE, '..'); + +function runReconcile(): { status: number | null; output: string } { + const require = createRequire(import.meta.url); + const tsx = require.resolve('tsx/cli'); + const result = spawnSync(process.execPath, [tsx, path.join(HERE, 'check-generated.ts'), '--reconcile-only'], { + cwd: SPEC, + encoding: 'utf8', + maxBuffer: 16 * 1024 * 1024, + }); + if (result.error) throw result.error; + return { status: result.status, output: `${result.stdout ?? ''}${result.stderr ?? ''}` }; +} + +describe('check:generated --reconcile-only', () => { + const scripts: Record = JSON.parse(fs.readFileSync(path.join(SPEC, 'package.json'), 'utf8')).scripts; + const checks = Object.keys(scripts).filter((n) => n.startsWith('check:')); + const gens = Object.keys(scripts).filter((n) => n.startsWith('gen:')); + + it('classifies every check:/gen: script this package declares', () => { + const { status, output } = runReconcile(); + // The failure text is the useful part when this goes red: it names the + // unclassified script and asks the classifying question. + expect(output).not.toMatch(/is in neither GATED nor NO_GENERATOR/); + expect(output).not.toMatch(/it is not in UNGATED_GENERATORS/); + expect(status, output).toBe(0); + }); + + it('reports the same script counts package.json actually declares', () => { + // Derived from package.json rather than hardcoded, so adding a gate does not + // churn this test — only FAILING to classify one does. + const { output } = runReconcile(); + expect(output).toContain(`${checks.length} check: + ${gens.length} gen: scripts`); + expect(output).toContain('all classified'); + }); + + it('covers the test-layer typecheck gate and its writer (#5286)', () => { + // The specific pair that failed CI on this branch. Named here so a later + // change that drops either script also has to come back through this file. + expect(scripts['check:test-typecheck']).toBeDefined(); + expect(scripts['gen:test-typecheck-debt']).toBeDefined(); + expect(runReconcile().status).toBe(0); + }); +}); diff --git a/packages/spec/scripts/check-generated.ts b/packages/spec/scripts/check-generated.ts index 17988de1e9..fcee97fda8 100644 --- a/packages/spec/scripts/check-generated.ts +++ b/packages/spec/scripts/check-generated.ts @@ -17,7 +17,8 @@ * never saw, so a real semantic change lands silently inside a mechanical diff. * What is worth automating is the *diagnosis* — which artifacts are stale, and * the exact command for each. `--fix` then regenerates **only** the ones this run - * proved stale, and says so. + * proved stale, and says so — minus the `ratchet` entries, whose gate has already + * answered a question `--fix` would otherwise have to guess (see GATED below). * * Usage: * pnpm --filter @objectstack/spec check:generated # report every stale artifact @@ -41,8 +42,18 @@ const pkgRoot = join(dirname(fileURLToPath(import.meta.url)), '..'); * The gates that verify a checked-in artifact against its source, with the * generator that rewrites each. Order is the cheapest-first order a human would * want the answers in, not CI's. + * + * `ratchet` marks the entries whose artifact is a DIRECTIONAL debt ledger rather + * than a descriptive snapshot of the source. For those, "stale" is ambiguous — + * see the `--fix` loop, which refuses to guess. */ -const GATED: ReadonlyArray<{ check: string; gen: string; artifact: string; readsDist?: true }> = [ +const GATED: ReadonlyArray<{ + check: string; + gen: string; + artifact: string; + readsDist?: true; + ratchet?: true; +}> = [ { check: 'check:spec-changes', gen: 'gen:spec-changes', artifact: 'spec-changes.json' }, { check: 'check:upgrade-guide', gen: 'gen:upgrade-guide', artifact: 'docs/protocol-upgrade-guide.md' }, { check: 'check:skill-docs', gen: 'gen:skill-docs', artifact: 'skill docs (from SKILL.md frontmatter)' }, @@ -69,6 +80,31 @@ const GATED: ReadonlyArray<{ check: string; gen: string; artifact: string; reads gen: 'gen:strictness-ledger', artifact: 'docs/audits/2026-07-unknown-key-strictness-ledger.counts.md', }, + // GATED by the definition above — it compares a checked-in artifact + // (test-typecheck-debt.json) against what `tsc -p tsconfig.test.json` measures + // right now, and `gen:test-typecheck-debt` is that artifact's writer. It is NOT + // a source audit: there is a real file to regenerate, so NO_GENERATOR would be + // a false classification, and UNGATED_GENERATORS ("nothing verifies this + // output") would be false in the other direction. + // + // What it is, that nothing above it is, is a DIRECTIONAL ratchet — hence + // `ratchet`. The other artifacts here are pure functions of the source, so + // regenerating is always the right answer. This one records DEBT, and its four + // verdicts split two ways: "the debt shrank" and "the file graduated" are + // re-record, while "the debt grew" and "an unledgered file has errors" are fix + // the code (#5286). `--fix` regenerates without reading which one it got, so + // for this entry it refuses instead — the merge that brought three new spec + // test files into this very branch is the live shape of the risk: had any of + // them carried errors, a reflexive `--fix` would have ledgered them silently. + // + // Cost: this is the only gate here that runs a full tsc program (~30s over + // src/**/*.test.ts), so it goes last in the cheapest-first order above. + { + check: 'check:test-typecheck', + gen: 'gen:test-typecheck-debt', + artifact: 'test-typecheck-debt.json', + ratchet: true, + }, ]; /** @@ -237,18 +273,54 @@ if (!stale.length) { } console.log(`\n✗ ${stale.length} of ${GATED.length} artifact(s) stale:\n`); -for (const s of stale) console.log(` ${s.artifact}\n pnpm --filter @objectstack/spec ${s.gen}`); +for (const s of stale) { + console.log(` ${s.artifact}\n pnpm --filter @objectstack/spec ${s.gen}` + + (s.ratchet ? ` ← only if ${s.check} asked you to RE-RECORD; --fix will not run this one` : '')); +} + +const autoFixable = stale.filter((s) => !s.ratchet); if (!fix) { console.log(`\nRegenerate exactly these:\n ` + - stale.map((s) => `pnpm --filter @objectstack/spec ${s.gen}`).join(' && ') + - `\n\nOr re-run with --fix to do it now (only the ${stale.length} proved stale — never the whole set).`); + stale.map((s) => `pnpm --filter @objectstack/spec ${s.gen}`).join(' && ')); + console.log( + autoFixable.length + ? `\nOr re-run with --fix to do it now (only the ${autoFixable.length} proved stale — never the whole set` + + (autoFixable.length < stale.length + ? `, and never the ${stale.length - autoFixable.length} ratchet(s) above: read their verdict first).` + : `).`) + : `\n--fix will not do this for you: every stale artifact above is a directional ratchet, ` + + `and its gate already said which direction it moved.`, + ); process.exit(1); } -console.log(`\n--fix: regenerating the ${stale.length} stale artifact(s). Review the diff before committing.\n`); +console.log( + `\n--fix: regenerating ${autoFixable.length} of the ${stale.length} stale artifact(s)` + + (autoFixable.length < stale.length ? ` — the rest are ratchets, refused below` : '') + + `. Review the diff before committing.\n`, +); let failed = 0; for (const s of stale) { + // A ratchet's gate has already answered the question --fix would have to guess: + // it names, per file, whether the debt grew (fix the code) or shrank (re-record + // the number). Regenerating on the first reading launders new debt in as a + // mechanical diff — the same "admit it via the fix command" hazard that keeps + // dual-source-exports.baseline.json out of GATED entirely (#4446). That ledger + // can stay hand-edited because it holds a handful of rows; this one holds 79 + // files, so it ships a generator and puts the refusal here instead. + if (s.ratchet) { + failed++; + console.log(` ✗ ${s.gen} — REFUSED`); + console.error( + ` ${s.artifact} is a directional debt ledger, not a snapshot of the source.\n` + + ` "the debt shrank — re-record it" and "the debt grew — fix the new errors" both\n` + + ` reach --fix as one stale artifact, and only ${s.check} knows which it was.\n` + + ` Read its per-file verdict; if re-recording is what it asked for, run:\n` + + ` pnpm --filter @objectstack/spec ${s.gen}`, + ); + continue; + } // The `readsDist` warning above is advice a reader can ignore; here it must // become a refusal. `gen:api-surface` on a stale dist does not fail — it // writes a plausible surface with every export added since the last build diff --git a/packages/spec/scripts/check-test-typecheck.mts b/packages/spec/scripts/check-test-typecheck.mts new file mode 100644 index 0000000000..ec9637767d --- /dev/null +++ b/packages/spec/scripts/check-test-typecheck.mts @@ -0,0 +1,305 @@ +#!/usr/bin/env tsx +// check-test-typecheck — the spec test layer is compiled by tsc, and every +// error it still carries is a named, measured, shrink-only entry (#5286). +// +// WHY THIS EXISTS. `packages/spec/tsconfig.json` excluded `**/*.test.ts`, and +// the package's `typecheck` script is `tsc --noEmit` against that very config. +// So no gate anywhere read a spec test file with a type checker: vitest +// transpiles through esbuild (types stripped, never resolved) and CI had no +// second compile step. Seventeen `@ts-expect-error` retirement pins across five +// files — the "tsc is the best sweeper" channel the spec-property-retirement +// playbook leans on — were PHANTOM checks. Deleting a directive line left the +// suite green, which is the definition of a check that never ran. +// +// The repair is `tsconfig.test.json`: the same strictness flags (they are +// inherited, untouched — this is fidelity, not loosening) with module semantics +// that match how vitest actually executes the files (`module: esnext`, +// `moduleResolution: bundler`, `lib` including ES2022). Under the build config's +// NodeNext, 108 of the 842 raw errors were the CHECK being misconfigured rather +// than the code being wrong (TS2835 x58 "dynamic import needs .js", TS1470 x24 +// `import.meta` in a CJS program, TS2307 x18, TS2550 x7 lib) — a config-tier +// pile that says nothing about the tests. Fixing the config first, then reading +// the residue, is the #4311 discipline this repo already writes down. +// +// The residue is real and large (691 errors over 79 files at the baseline), +// overwhelmingly fixture object literals annotated with a schema's OUTPUT type +// (`z.infer`) while holding an authored INPUT literal — so every defaulted key +// reads as "missing". Hand-fixing 691 of those in the PR that opens the gate +// would bury the gate. They are ledgered per file instead, in +// `test-typecheck-debt.json`, and the ledger is EXACT: recorded must equal +// measured. That is what makes it shrink-only in practice — +// +// • a file gains errors → red ("grew") +// • a file loses errors → red ("shrank; re-record") — so the number +// tracks reality downward instead of rotting +// • a file reaches zero → red ("graduated; delete the entry") +// • an unledgered file errors→ red — this is the everyday case, and it is +// why the five pin files carry NO entry: any +// error in them, including the TS2578 that a +// deleted `@ts-expect-error` produces, is red. +// +// Growing the ledger is possible (add the file and its count) but it is a +// visible line in this repo's diff and needs the same justification any DEBT +// entry needs — the idiom of `scripts/check-type-check-coverage.mjs`, applied +// per file rather than per package. +// +// Usage: +// tsx scripts/check-test-typecheck.mts # compile + judge +// tsx scripts/check-test-typecheck.mts --update # re-record the ledger +// tsx scripts/check-test-typecheck.mts --self-test # ledger semantics only + +import { spawnSync } from 'node:child_process'; +import { createRequire } from 'node:module'; +import fs from 'node:fs'; +import path from 'node:path'; +import url from 'node:url'; + +const HERE = path.dirname(url.fileURLToPath(import.meta.url)); +const SPEC = path.resolve(HERE, '..'); +// Named on the command line rather than hardcoded, so the wiring is visible in +// package.json: `check:type-check-coverage` reads the typecheck script chain to +// decide whether a sibling test tsconfig is actually invoked, and a config no +// script names is exactly the phantom this gate is about. +const PROJECT = ((): string => { + const i = process.argv.indexOf('--project'); + return i !== -1 && process.argv[i + 1] ? process.argv[i + 1] : 'tsconfig.test.json'; +})(); +const LEDGER_PATH = path.join(SPEC, 'test-typecheck-debt.json'); +const LEDGER_NAME = 'test-typecheck-debt.json'; +const ISSUE = 'https://github.com/objectstack-ai/objectstack/issues/5286'; +const UPDATE_COMMAND = 'pnpm --filter @objectstack/spec gen:test-typecheck-debt'; + +const LEDGER_COMMENT = + 'Per-file tsc error debt of the @objectstack/spec TEST layer (#5286). ' + + '`tsconfig.test.json` compiles `src/**/*.test.ts` — which `tsconfig.json` excludes and therefore ' + + 'no gate ever read — and every file below still carries errors from before that gate existed, ' + + 'almost all of them fixture literals annotated with a schema OUTPUT type (`z.infer`) while holding ' + + 'an authored INPUT literal. EXACT ratchet, judged by re-running tsc: a file that gains errors is red, ' + + 'a file that loses them is red until its number is re-recorded, a file that reaches zero is red until ' + + 'its entry is deleted, and a file NOT listed here may have no errors at all. Regenerate with: ' + + UPDATE_COMMAND; + +type Ledger = { _comment: string; entries: Record }; + +/** `path(line,col): error TSxxxx: …` — continuation lines of a multi-line message never match. */ +const DIAGNOSTIC = /^(\S[^(]*)\((\d+),(\d+)\): error (TS\d+): /; + +/** + * Per-file error counts from a raw `tsc --noEmit --pretty false` transcript. + * Paths are normalised to posix and relative to the spec package, so the ledger + * reads the same on every platform. + */ +export function parseDiagnostics(output: string): Map { + const counts = new Map(); + for (const line of output.split(/\r?\n/)) { + const m = DIAGNOSTIC.exec(line); + if (!m) continue; + const file = m[1].split(path.sep).join('/'); + counts.set(file, (counts.get(file) ?? 0) + 1); + } + return counts; +} + +/** + * The verdict, as a pure function over observed counts and the recorded ledger, + * so the self-test proves the semantics the real run applies. + */ +export function evaluate(actual: Map, ledger: Record): string[] { + const problems: string[] = []; + + for (const [file, count] of [...actual].sort(([a], [b]) => a.localeCompare(b))) { + if (!Object.hasOwn(ledger, file)) { + problems.push( + `${file}: ${count} type error(s) in a file the ledger does not cover. Fix them — this file is ` + + `inside the checked zone, which is the point of ${PROJECT}. (A deleted \`@ts-expect-error\` ` + + `shows up exactly here, as TS2578/TS2694.) Only with a reason: add the file to ${LEDGER_NAME}.`, + ); + continue; + } + const recorded = ledger[file]; + if (count > recorded) { + problems.push( + `${file}: ${count} type error(s), ledger records ${recorded} — the debt GREW. Fix the ${count - recorded} ` + + `new one(s); the ledger only ratchets down (${ISSUE}).`, + ); + } else if (count < recorded) { + problems.push( + `${file}: ${count} type error(s), ledger records ${recorded} — the debt SHRANK, which is the goal. ` + + `Re-record it so the number stays true: ${UPDATE_COMMAND}.`, + ); + } + } + + for (const [file, recorded] of Object.entries(ledger).sort(([a], [b]) => a.localeCompare(b))) { + if (typeof recorded !== 'number' || !Number.isInteger(recorded) || recorded <= 0) { + problems.push( + `${file}: ledger entry is not a positive integer error count (${JSON.stringify(recorded)}) — ` + + `a ledger without a measurement is a permission slip.`, + ); + continue; + } + if (!actual.has(file)) { + problems.push( + `${file}: ledger records ${recorded} type error(s) but tsc reports none — it GRADUATED, or the file ` + + `moved/vanished. Delete its entry from ${LEDGER_NAME} in the same change.`, + ); + } + } + + return problems; +} + +function loadLedger(): Ledger { + if (!fs.existsSync(LEDGER_PATH)) return { _comment: LEDGER_COMMENT, entries: {} }; + const parsed = JSON.parse(fs.readFileSync(LEDGER_PATH, 'utf8')) as Ledger; + if (!parsed || typeof parsed !== 'object' || typeof parsed.entries !== 'object') { + throw new Error(`${LEDGER_NAME} is not { _comment, entries } — refusing to judge against an unreadable ledger.`); + } + return parsed; +} + +function writeLedger(counts: Map): void { + const entries: Record = {}; + for (const file of [...counts.keys()].sort()) entries[file] = counts.get(file)!; + const ledger: Ledger = { _comment: LEDGER_COMMENT, entries }; + fs.writeFileSync(LEDGER_PATH, `${JSON.stringify(ledger, null, 2)}\n`, 'utf8'); +} + +/** Compile the test program and hand back the raw transcript. */ +function runTsc(): string { + const require = createRequire(import.meta.url); + const tsc = require.resolve('typescript/bin/tsc'); + const result = spawnSync(process.execPath, [tsc, '--noEmit', '--pretty', 'false', '-p', PROJECT], { + cwd: SPEC, + encoding: 'utf8', + maxBuffer: 64 * 1024 * 1024, + env: { ...process.env, NODE_OPTIONS: process.env.NODE_OPTIONS ?? '--max-old-space-size=4096' }, + }); + if (result.error) throw result.error; + const output = `${result.stdout ?? ''}${result.stderr ?? ''}`; + // A non-zero exit with NO parseable diagnostic means tsc could not run at all + // (missing config, crash, OOM). That must not read as "no errors found" — the + // failure mode this whole file exists to prevent. + const hasDiagnostics = output.split(/\r?\n/).some((line) => DIAGNOSTIC.test(line)); + if (result.status !== 0 && !hasDiagnostics) { + throw new Error(`tsc -p ${PROJECT} failed without diagnostics (exit ${result.status}):\n${output}`); + } + return output; +} + +function selfTest(): void { + const cases: Array<{ label: string; actual: Array<[string, number]>; ledger: Record; expect: RegExp[] }> = [ + { + label: 'a clean, unledgered file passes', + actual: [], + ledger: {}, + expect: [], + }, + { + label: 'a ledgered file at its recorded count passes', + actual: [['a.test.ts', 3]], + ledger: { 'a.test.ts': 3 }, + expect: [], + }, + { + label: 'an error in an unledgered file is red — the everyday case, and what a deleted directive produces', + actual: [['pin.test.ts', 1]], + ledger: {}, + expect: [/pin\.test\.ts: 1 type error\(s\) in a file the ledger does not cover/], + }, + { + label: 'a ledgered file that gains an error is red', + actual: [['a.test.ts', 4]], + ledger: { 'a.test.ts': 3 }, + expect: [/a\.test\.ts: 4 type error\(s\), ledger records 3 — the debt GREW/], + }, + { + label: 'a ledgered file that loses an error is red until re-recorded', + actual: [['a.test.ts', 2]], + ledger: { 'a.test.ts': 3 }, + expect: [/a\.test\.ts: 2 type error\(s\), ledger records 3 — the debt SHRANK/], + }, + { + label: 'a graduated file is red until its entry is deleted', + actual: [], + ledger: { 'a.test.ts': 3 }, + expect: [/a\.test\.ts: ledger records 3 type error\(s\) but tsc reports none/], + }, + { + label: 'a ledger entry without a real measurement is red', + actual: [], + ledger: { 'a.test.ts': 0 }, + expect: [/a\.test\.ts: ledger entry is not a positive integer error count/], + }, + { + label: 'problems from both directions are reported together', + actual: [ + ['a.test.ts', 4], + ['new.test.ts', 1], + ], + ledger: { 'a.test.ts': 3, 'gone.test.ts': 2 }, + expect: [ + /a\.test\.ts: 4 type error\(s\), ledger records 3 — the debt GREW/, + /new\.test\.ts: 1 type error\(s\) in a file the ledger does not cover/, + /gone\.test\.ts: ledger records 2 type error\(s\) but tsc reports none/, + ], + }, + ]; + + const failures: string[] = []; + for (const c of cases) { + const got = evaluate(new Map(c.actual), c.ledger); + if (got.length !== c.expect.length || !c.expect.every((rx, i) => rx.test(got[i]))) { + failures.push(`${c.label}: expected ${c.expect.length} problem(s) matching ${c.expect}, got ${JSON.stringify(got)}`); + } + } + + // The parser is the other half of the semantics: a multi-line tsc message + // must count once, and a continuation line must never be read as a file. + const parsed = parseDiagnostics( + [ + "src/a.test.ts(1,1): error TS2739: Type '{}' is missing the following properties from type 'X':", + " Type 'string[]' is not assignable to type 'Y'.", + "src/a.test.ts(9,3): error TS2578: Unused '@ts-expect-error' directive.", + 'src/b.test.ts(2,2): error TS2304: Cannot find name.', + '', + ].join('\n'), + ); + if (parsed.size !== 2 || parsed.get('src/a.test.ts') !== 2 || parsed.get('src/b.test.ts') !== 1) { + failures.push(`parseDiagnostics mis-read a multi-line transcript: ${JSON.stringify([...parsed])}`); + } + + if (failures.length) { + console.error(`✗ check:test-typecheck --self-test — ${failures.length} failure(s)\n`); + for (const f of failures) console.error(' • ' + f); + process.exit(1); + } + console.log(`✓ check:test-typecheck --self-test — ${cases.length} semantic case(s) + the parser hold.`); +} + +if (process.argv.includes('--self-test')) { + selfTest(); + process.exit(0); +} + +const counts = parseDiagnostics(runTsc()); + +if (process.argv.includes('--update')) { + writeLedger(counts); + const total = [...counts.values()].reduce((a, b) => a + b, 0); + console.log(`check:test-typecheck — re-recorded ${LEDGER_NAME}: ${counts.size} file(s), ${total} error(s).`); + process.exit(0); +} + +const problems = evaluate(counts, loadLedger().entries); +if (problems.length) { + console.error(`check:test-typecheck: ${problems.length} problem(s)\n`); + for (const p of problems) console.error(' • ' + p); + process.exit(1); +} + +const total = [...counts.values()].reduce((a, b) => a + b, 0); +console.log( + `check:test-typecheck: OK — spec's test layer compiles under ${PROJECT}; ` + + `${counts.size} file(s) / ${total} error(s) held in ${LEDGER_NAME} (shrink-only, ${ISSUE}).`, +); diff --git a/packages/spec/src/api/endpoint.test.ts b/packages/spec/src/api/endpoint.test.ts index 69091720b8..022e20427d 100644 --- a/packages/spec/src/api/endpoint.test.ts +++ b/packages/spec/src/api/endpoint.test.ts @@ -1,4 +1,8 @@ import { describe, it, expect } from 'vitest'; +// `z` is used in a type position below (`z.infer`) and was +// never imported — TS2503, invisible to vitest because esbuild strips type +// annotations without resolving them (#5286). +import type { z } from 'zod'; import { ApiEndpointSchema, ApiMappingSchema, diff --git a/packages/spec/src/api/registry-retirement.test.ts b/packages/spec/src/api/registry-retirement.test.ts index 1a42a58c8a..404ab5264f 100644 --- a/packages/spec/src/api/registry-retirement.test.ts +++ b/packages/spec/src/api/registry-retirement.test.ts @@ -28,8 +28,8 @@ * * A removed export cannot be imported by name — that would not compile, so the * pin has to ask the namespace object instead. `#4642` established that a - * compile-time conditional-type pin is a no-op in this package (tsconfig - * excludes `**\/*.test.ts`, and vitest never enables `typecheck`), so the + * compile-time conditional-type pin was a no-op until #5286 in this package (tsconfig + * excluded `**\/*.test.ts`, and vitest never enables `typecheck`), so the * load-bearing check must be a runtime one, with anti-vacuity guards. */ diff --git a/packages/spec/src/api/rest-server.test.ts b/packages/spec/src/api/rest-server.test.ts index b7aa8221c5..24d92a0222 100644 --- a/packages/spec/src/api/rest-server.test.ts +++ b/packages/spec/src/api/rest-server.test.ts @@ -693,8 +693,8 @@ describe('[#4579] `RestServerConfig.openApi31` retirement', () => { }); }); -// #4642 established that a compile-time conditional-type pin in this package is -// a no-op (tsconfig excludes `**/*.test.ts`; vitest never enables `typecheck`), +// #4642 established that a compile-time conditional-type pin in this package was +// a no-op until #5286 (tsconfig excluded `**/*.test.ts`; vitest never enables `typecheck`), // so the load-bearing pin is the compiler-API test below, with anti-vacuity // guards (a resolution failure would otherwise make every assertion pass // vacuously); sabotage-verified in the PR (re-adding an export turns it red). diff --git a/packages/spec/src/api/router.test.ts b/packages/spec/src/api/router.test.ts index 0c0ba5ffe4..447cc5ddf0 100644 --- a/packages/spec/src/api/router.test.ts +++ b/packages/spec/src/api/router.test.ts @@ -1,4 +1,8 @@ import { describe, it, expect } from 'vitest'; +// `z` is used in a type position below (`z.infer`) and was +// never imported — TS2503, invisible to vitest because esbuild strips type +// annotations without resolving them (#5286). +import type { z } from 'zod'; import { RouteCategory, RouteDefinitionSchema, diff --git a/packages/spec/src/automation/state-machine.test.ts b/packages/spec/src/automation/state-machine.test.ts index 8ad0962380..546fca0278 100644 --- a/packages/spec/src/automation/state-machine.test.ts +++ b/packages/spec/src/automation/state-machine.test.ts @@ -328,7 +328,7 @@ describe('[#4001] ActionRef / GuardRef — strict inside a union', () => { // signal *declaration* already exists: `EventTypeDefinitionSchema`, same file. // // #4642 established that a compile-time conditional-type pin in this package -// is a no-op (tsconfig excludes `**/*.test.ts`; vitest never enables +// was a no-op until #5286 (tsconfig excluded `**/*.test.ts`; vitest never enables // `typecheck`), so the load-bearing pin is the compiler-API test below, with // anti-vacuity guards; sabotage-verified in the PR (re-adding the export // turns it red). diff --git a/packages/spec/src/automation/sync-retirement.test.ts b/packages/spec/src/automation/sync-retirement.test.ts index f791ab6f45..bfb7111de6 100644 --- a/packages/spec/src/automation/sync-retirement.test.ts +++ b/packages/spec/src/automation/sync-retirement.test.ts @@ -45,7 +45,7 @@ import { describe, it, expect } from 'vitest'; // and must not be touched by any of this. // // #4642 established that a compile-time conditional-type pin in this package -// is a no-op (tsconfig excludes `**/*.test.ts`; vitest never enables +// was a no-op until #5286 (tsconfig excluded `**/*.test.ts`; vitest never enables // `typecheck`), so the load-bearing pin is the compiler-API test below, with // anti-vacuity guards; sabotage-verified in the PR (re-adding an automation // export, re-introducing a bare-name re-export on ./integration, and renaming diff --git a/packages/spec/src/cloud/tenant.test.ts b/packages/spec/src/cloud/tenant.test.ts index 2387404307..9fb2089c86 100644 --- a/packages/spec/src/cloud/tenant.test.ts +++ b/packages/spec/src/cloud/tenant.test.ts @@ -30,7 +30,7 @@ import { TenantPlanSchema } from './tenant.zod'; // not flag it — the uniqueness pin below catches exactly that (S2 sabotage). // // #4642 established that a compile-time conditional-type pin in this package -// is a no-op (tsconfig excludes `**/*.test.ts`; vitest never enables +// was a no-op until #5286 (tsconfig excluded `**/*.test.ts`; vitest never enables // `typecheck`), so the load-bearing pin is the compiler-API test below, with // anti-vacuity guards; sabotage-verified in the PR. describe('[#4739] `TenantPlan(Schema)` resolves to the ./cloud declaration everywhere', () => { diff --git a/packages/spec/src/data/driver.test.ts b/packages/spec/src/data/driver.test.ts index 76f22134f7..b7a7cff9c3 100644 --- a/packages/spec/src/data/driver.test.ts +++ b/packages/spec/src/data/driver.test.ts @@ -122,8 +122,8 @@ describe('[#4634] the 31 inert capability bits are tombstoned, not stripped', () }); }); -// #4642 established that a compile-time conditional-type pin in this package is -// a no-op (tsconfig excludes `**/*.test.ts`; vitest never enables `typecheck`), +// #4642 established that a compile-time conditional-type pin in this package was +// a no-op until #5286 (tsconfig excluded `**/*.test.ts`; vitest never enables `typecheck`), // so the load-bearing tsc-channel proof is the compiler-API test below, with // anti-vacuity guards; sabotage-verified in the PR (S1: re-adding a live // `streaming: z.boolean()` turns it red). diff --git a/packages/spec/src/data/filter-array-declaration.test.ts b/packages/spec/src/data/filter-array-declaration.test.ts index 41f640a11c..fb91f607de 100644 --- a/packages/spec/src/data/filter-array-declaration.test.ts +++ b/packages/spec/src/data/filter-array-declaration.test.ts @@ -204,31 +204,33 @@ describe('the authoring gate is stricter than the runtime detector, in exactly t }); /** - * ⚠️ **These assertions do not run in CI, and saying so is the point.** + * **These assertions DO run in CI now — #5286 is what made that true.** * - * `packages/spec/tsconfig.json` excludes `**` + `/*.test.ts` under the measured - * `TEST_DEBT` entry in `scripts/check-type-check-coverage.mjs` (272 test files, - * 902 errors), so `pnpm --filter @objectstack/spec typecheck` never reads this - * file and every `@ts-expect-error` below is INERT — it looks like a pinned - * contract and pins nothing. That is true of all 17 `@ts-expect-error` - * directives across spec's test layer, not just these two; filed as #5305. + * The history is worth keeping, because it is the whole reason the gate exists. + * `packages/spec/tsconfig.json` excludes `**` + `/*.test.ts` and the package's + * `typecheck` script was a bare `tsc --noEmit` reading that same config, so + * `pnpm --filter @objectstack/spec typecheck` never read this file and every + * `@ts-expect-error` below was INERT — it looked like a pinned contract and + * pinned nothing. That was true of all 17 `@ts-expect-error` directives across + * spec's test layer, not just these two (#5305, folded into #5286). * - * They are kept because they are correct and become live the day spec - * graduates off `TEST_DEBT`. Verified by hand on this branch, with the - * exclusion lifted: + * The build config still excludes tests, on purpose: ci.yml gates that no test + * file reaches the published artifact. What changed is the sibling + * `tsconfig.test.json`, named by the `typecheck` script, which compiles the test + * layer with the module semantics vitest actually executes under. Two gates now + * keep this honest: `check:test-typecheck` fails on any error in a file the + * per-file ledger does not cover (these two directives going unused is exactly + * such an error), and `check:type-check-coverage`'s PINS_CHECKED invariant fails + * repo-wide on a `@ts-expect-error` sitting outside every tsc program. * - * ``` - * # tsconfig extending spec's, "include": [this file], "exclude": [] - * npx tsc -p tsconfig.typetest.tmp.json # => exit 0, both directives live - * ``` - * - * and reverse-verified by widening `FilterArrayOperator` back to `string` - * (restoring the `Record< string, string >` annotation on `AST_OPERATOR_MAP`), - * which reports exactly one new error — `TS2578: Unused '@ts-expect-error' - * directive` on the misspelled-operator line, the narrowing this declaration - * adds. Do not read a green `pnpm test` as evidence for anything in this block. + * Reverse-verified in the #5286 PR by widening `FilterArrayOperator` back to + * `string` (restoring the `Record< string, string >` annotation on + * `AST_OPERATOR_MAP`), which reports exactly one new error — `TS2578: Unused + * '@ts-expect-error' directive` on the misspelled-operator line, the narrowing + * this declaration adds. A green `pnpm test` is still not evidence for this + * block; `pnpm typecheck` now is. */ -describe('FilterArray type-level declaration (NOT type-checked in CI — see above)', () => { +describe('FilterArray type-level declaration (type-checked since #5286 — see above)', () => { it('narrows the operator position to the canonical vocabulary', () => { const canonical: FilterArrayOperator = 'starts_with'; const comparison: FilterArray = ['name', canonical, 'A']; diff --git a/packages/spec/src/data/hook.test.ts b/packages/spec/src/data/hook.test.ts index becb98be8e..f44423bb3f 100644 --- a/packages/spec/src/data/hook.test.ts +++ b/packages/spec/src/data/hook.test.ts @@ -469,7 +469,9 @@ describe('HookContextSchema', () => { ql: {}, }); - expect(context.input.doc.name).toBe('New Account'); + // `input` is `z.record(z.string(), z.unknown())` by contract — the payload + // shape varies per event — so a parsed read is narrowed at the read site. + expect((context.input.doc as { name: string }).name).toBe('New Account'); }); it('should accept update input', () => { @@ -485,7 +487,7 @@ describe('HookContextSchema', () => { }); expect(context.input.id).toBe('123'); - expect(context.input.doc.status).toBe('active'); + expect((context.input.doc as { status: string }).status).toBe('active'); }); it('should accept delete input', () => { @@ -517,7 +519,7 @@ describe('HookContextSchema', () => { ql: {}, }); - expect(context.result.id).toBe('123'); + expect((context.result as { id: string }).id).toBe('123'); }); it('should accept array result', () => { @@ -671,7 +673,12 @@ describe('HookContextSchema', () => { }, session: { userId: 'user_123', - tenantId: 'tenant_456', + // `session.tenantId` was removed in v11 (#3280/#3290); the blessed + // developer-facing name is `organizationId`. This fixture kept + // spelling the retired alias for two majors because nothing + // type-checked it — vitest only sees `HookContextSchema.parse`, + // which strips unknown keys silently (#5286). + organizationId: 'org_456', roles: ['user'], }, transaction: { id: 'tx_789' }, @@ -760,7 +767,7 @@ describe('Integration Tests', () => { expect(hook.events).toContain('beforeInsert'); expect(beforeContext.event).toBe('beforeInsert'); - expect(afterContext.result.id).toBe('123'); + expect((afterContext.result as { id: string }).id).toBe('123'); }); }); diff --git a/packages/spec/src/data/object.test.ts b/packages/spec/src/data/object.test.ts index 13ef210757..31fe80a2c3 100644 --- a/packages/spec/src/data/object.test.ts +++ b/packages/spec/src/data/object.test.ts @@ -1,5 +1,13 @@ import { describe, it, expect, vi, afterEach } from 'vitest'; -import { ObjectSchema, ObjectCapabilities, IndexSchema, ObjectFieldGroupSchema, ObjectExternalBindingSchema, ObjectAccessConfigSchema, LifecycleSchema, TenancyConfigSchema, isTenancyDisabled, resolveCrudAffordances, type ServiceObject } from './object.zod'; +// Fixtures below are AUTHORED objects — what a developer writes before the +// schema applies its defaults — so they are annotated with `ServiceObjectInput` +// (`z.input`), not `ServiceObject` (`z.infer`, defaults already materialised). +// Under `z.infer` every fixture owes `isSystem`, `datasource`, `searchable`, +// `activities`, … and the annotation stops being a contract check at all. This +// only became visible when tsconfig.test.json put these files in front of tsc +// (#5286). +import { ObjectSchema, ObjectCapabilities, IndexSchema, ObjectFieldGroupSchema, ObjectExternalBindingSchema, ObjectAccessConfigSchema, LifecycleSchema, TenancyConfigSchema, isTenancyDisabled, resolveCrudAffordances, type ServiceObjectInput } from './object.zod'; +import type { StateMachineValidation } from './validation.zod'; describe('ObjectCapabilities', () => { it('should apply default values correctly', () => { @@ -224,7 +232,7 @@ describe('IndexSchema', () => { describe('ObjectSchema', () => { describe('Basic Object Properties', () => { it('should accept minimal valid object', () => { - const validObject: ServiceObject = { + const validObject: ServiceObjectInput = { name: 'account', fields: {}, }; @@ -259,7 +267,7 @@ describe('ObjectSchema', () => { describe('Object with Fields', () => { it('should accept object with multiple fields', () => { - const objectWithFields: ServiceObject = { + const objectWithFields: ServiceObjectInput = { name: 'contact', label: 'Contact', pluralLabel: 'Contacts', @@ -417,7 +425,7 @@ describe('ObjectSchema', () => { describe('Object Metadata', () => { it('should accept object with full metadata', () => { - const fullObject: ServiceObject = { + const fullObject: ServiceObjectInput = { name: 'opportunity', label: 'Opportunity', pluralLabel: 'Opportunities', @@ -440,7 +448,7 @@ describe('ObjectSchema', () => { describe('Object with Indexes', () => { it('should accept object with indexes', () => { - const objectWithIndexes: ServiceObject = { + const objectWithIndexes: ServiceObjectInput = { name: 'user', fields: { email: { @@ -475,7 +483,7 @@ describe('ObjectSchema', () => { describe('Object Capabilities', () => { it('should accept object with custom capabilities', () => { - const objectWithCapabilities: ServiceObject = { + const objectWithCapabilities: ServiceObjectInput = { name: 'case', fields: {}, enable: { @@ -510,7 +518,7 @@ describe('ObjectSchema', () => { describe('Complete Real-World Examples', () => { it('should accept CRM Account object', () => { - const accountObject: ServiceObject = { + const accountObject: ServiceObjectInput = { name: 'account', label: 'Account', pluralLabel: 'Accounts', @@ -575,7 +583,7 @@ describe('ObjectSchema', () => { }); it('should accept Task object with parent relationship', () => { - const taskObject: ServiceObject = { + const taskObject: ServiceObjectInput = { name: 'task', label: 'Task', pluralLabel: 'Tasks', @@ -663,13 +671,17 @@ describe('ObjectSchema', () => { }; const result = ObjectSchema.parse(objectWithState); - const rule = result.validations!.find((v) => v.name === 'leave_flow'); + // `validations` is typed as the open `BaseValidationRuleShape` (index + // signature, see validation.zod.ts) — the discriminated union does the + // real rejecting at parse time. Narrow to the member this test is about + // instead of asserting through a shape that does not overlap it. + const rule = result.validations!.find((v) => v.name === 'leave_flow') as + | StateMachineValidation + | undefined; expect(rule).toBeDefined(); expect(rule!.type).toBe('state_machine'); - expect((rule as { field: string }).field).toBe('status'); - expect((rule as { transitions: Record }).transitions.draft).toEqual([ - 'pending', - ]); + expect(rule!.field).toBe('status'); + expect(rule!.transitions.draft).toEqual(['pending']); }); it('should allow multiple state_machine rules over distinct fields', () => { diff --git a/packages/spec/src/identity/identity.test.ts b/packages/spec/src/identity/identity.test.ts index 7e47068bf4..c66da9d706 100644 --- a/packages/spec/src/identity/identity.test.ts +++ b/packages/spec/src/identity/identity.test.ts @@ -151,12 +151,14 @@ describe('Session is not declared here (#4641)', () => { // (`expires` vs `expiresAt`, `sessionToken` vs `token`). // // Why this asserts at RUNTIME rather than with the `typeof import(...)` - // conditional-type pin used by #4581 / #4610: that pin cannot fail here. - // `packages/spec/tsconfig.json` excludes `**/*.test.ts`, so `pnpm typecheck` - // never compiles this file, and vitest transpiles without typechecking — a - // conditional-type assertion in a spec test is inert. (Filed separately; it - // silently weakens the pins those two PRs landed.) The module-namespace check - // below is cheap — this file already imports the module — and it actually runs. + // conditional-type pin used by #4581 / #4610: when this was written that pin + // could not fail here. `packages/spec/tsconfig.json` excluded `**/*.test.ts`, + // so `pnpm typecheck` never compiled this file, and vitest transpiles without + // typechecking — a conditional-type assertion in a spec test was inert. + // #5286 closed that hole (the sibling `tsconfig.test.json` compiles the test + // layer, and PINS_CHECKED keeps it that way). The module-namespace check + // below is cheap — this file already imports the module — and it actually + // runs, which is why it stays the load-bearing one. // // Scope, deliberately: this covers the VALUE export. A type-only // `export type Session` has no runtime footprint, so no unit test can see it. diff --git a/packages/spec/src/integration/connector.test.ts b/packages/spec/src/integration/connector.test.ts index 852580556c..b21a008fa6 100644 --- a/packages/spec/src/integration/connector.test.ts +++ b/packages/spec/src/integration/connector.test.ts @@ -666,9 +666,9 @@ describe('ConnectorHealthSchema', () => { // ─── [#4911] Outbound rate limiting retired — with the [#4684] pin folded in ── // // RUNTIME + compiler-API assertions, deliberately. #4642 established that a -// compile-time pin in `packages/spec` is a no-op: `tsconfig.json` excludes +// compile-time pin in `packages/spec` was a no-op until #5286: `tsconfig.json` excluded // `**/*.test.ts` and `vitest.config.ts` never enables `typecheck`, so an -// `Assert< Equal< … > >` here would be dead text. The last test below is the +// `Assert< Equal< … > >` here was dead text until #5286. The last test below is the // only shape in this repo that actually pins a TYPE — which is what makes it the // load-bearing one here: `ConnectorRateLimitConfig` is a TYPE, erased before any // runtime assertion can see it, so re-adding `export type ConnectorRateLimitConfig` @@ -1048,8 +1048,8 @@ describe('[#4703] FieldMapping no longer names three declarations', () => { // The load-bearing one, and the reason this block exists at all: `FieldMapping` // is a TYPE. Every runtime assertion above stays green if any entry re-adds // `export type FieldMapping = z.infer<…>`, which IS the defect. #4642 proved a - // compile-time `Assert< Equal< … > >` is dead text in this package - // (`tsconfig.json` excludes `**/*.test.ts`, vitest never enables `typecheck`), + // compile-time `Assert< Equal< … > >` was dead text until #5286 in this package + // (`tsconfig.json` excluded `**/*.test.ts`, vitest never enables `typecheck`), // so this resolves each entry's exports through their alias chains with the // TypeScript compiler API — the same symbol-identity measurement // `check:dual-source-exports` makes against `dist`, run over `src/` so it is diff --git a/packages/spec/src/kernel/activation-events-retirement.test.ts b/packages/spec/src/kernel/activation-events-retirement.test.ts index a928f6548b..f9322b3ae5 100644 --- a/packages/spec/src/kernel/activation-events-retirement.test.ts +++ b/packages/spec/src/kernel/activation-events-retirement.test.ts @@ -18,8 +18,8 @@ import { describe, it, expect } from 'vitest'; // re-export #4653 had just added) are deleted. // // Why a compiler-API pin rather than a type-level one: #4642 established that -// a compile-time conditional-type pin in this package is a no-op (tsconfig -// excludes `**/*.test.ts`; vitest never enables `typecheck`), so the +// a compile-time conditional-type pin in this package was a no-op until #5286 (tsconfig +// excluded `**/*.test.ts`; vitest never enables `typecheck`), so the // load-bearing pin is the program below, with anti-vacuity guards — the // #4737 `ActionLocation` retirement's machinery, pointed at absence instead of // ownership. Sabotage-verified in the PR: resurrecting the declaration in diff --git a/packages/spec/src/kernel/package-artifact.test.ts b/packages/spec/src/kernel/package-artifact.test.ts index d78e2eec99..3c097cae66 100644 --- a/packages/spec/src/kernel/package-artifact.test.ts +++ b/packages/spec/src/kernel/package-artifact.test.ts @@ -151,7 +151,10 @@ describe('PackageArtifactSchema', () => { { path: 'data/seed.json', size: 8192 }, { path: 'locales/en.json', size: 256 }, ], - metadataCategories: ['objects', 'views'] as const[], + // Was `as const[]` — which TypeScript reads as "array of the type named + // `const`", i.e. TS2304 "Cannot find name 'const'". A typo that survived + // because esbuild strips the annotation without resolving it (#5286). + metadataCategories: ['objects', 'views'], checksums: { algorithm: 'sha256' as const, files: { diff --git a/packages/spec/src/kernel/package-dependency-dual-source.test.ts b/packages/spec/src/kernel/package-dependency-dual-source.test.ts index 5eeb66dfff..53804f63e9 100644 --- a/packages/spec/src/kernel/package-dependency-dual-source.test.ts +++ b/packages/spec/src/kernel/package-dependency-dual-source.test.ts @@ -27,8 +27,8 @@ import { describe, it, expect } from 'vitest'; // `kernel/ResolvedPackageDependency` (4 authorable keys, all four still // present under the new def ⇒ zero tombstone, zero ADR-0087 conversion). // -// #4642 established that a compile-time conditional-type pin in this package is -// a no-op (tsconfig excludes `**/*.test.ts`; vitest never enables `typecheck`), +// #4642 established that a compile-time conditional-type pin in this package was +// a no-op until #5286 (tsconfig excluded `**/*.test.ts`; vitest never enables `typecheck`), // so the load-bearing pin is the compiler-API test below, with anti-vacuity // guards. Sabotage-verified in the PR: S1 resurrecting the old kernel const, // and S2 re-exporting the cloud declaration from ./kernel under the bare name diff --git a/packages/spec/src/kernel/plugin-runtime-retirement.test.ts b/packages/spec/src/kernel/plugin-runtime-retirement.test.ts index b1cac906dd..61764f65d4 100644 --- a/packages/spec/src/kernel/plugin-runtime-retirement.test.ts +++ b/packages/spec/src/kernel/plugin-runtime-retirement.test.ts @@ -15,10 +15,13 @@ import { describe, it, expect } from 'vitest'; // paragraph; #4834 is that decision, answered. // // Why THIS pin, and not a type-level one: #4642 established that a -// compile-time conditional-type assertion in this package is a no-op (the -// package tsconfig excludes `**/*.test.ts`, and vitest never enables -// `typecheck`), so a `Assert< Equal< … > >` here would be decoration. The -// load-bearing pin is the TypeScript compiler-API program below, which resolves +// compile-time conditional-type assertion in this package was a no-op (the +// package tsconfig excluded `**/*.test.ts`, and vitest never enables +// `typecheck`). #5286 closed that hole — `tsconfig.test.json` now compiles this +// file — so a type-level assertion here would no longer be dead text. It stays +// a runtime pin anyway, because a conditional over `keyof typeof import(...)` +// only enumerates VALUE exports (#4642) and this retirement covers types too. +// The load-bearing pin is the TypeScript compiler-API program below, which resolves // the REAL export surface of EVERY public entry from `package.json`'s exports // map and asserts each retired name has zero holders — by symbol identity, not // by grepping text. @@ -135,6 +138,13 @@ describe('[#4834] plugin-runtime family removal — no entry exports any of the // The names could be absent from the barrels while the module still sat on // disk, importable by deep path and still emitting json-schema defs. It // does not. - await expect(import('./plugin-runtime.zod')).rejects.toThrow(); + // + // The specifier is held in a variable on purpose. Now that tsc compiles + // this file (#5286), a literal `import('./plugin-runtime.zod')` is a + // TS2307 "cannot find module" — the compiler is agreeing with the test and + // failing the build for it. An indirect specifier keeps the RUNTIME + // assertion (the load must reject) exactly as it was, which is the pin. + const retiredModule = './plugin-runtime.zod'; + await expect(import(retiredModule)).rejects.toThrow(); }); }); diff --git a/packages/spec/src/shared/retry-policy.test.ts b/packages/spec/src/shared/retry-policy.test.ts index 169f887c73..118d50fcc0 100644 --- a/packages/spec/src/shared/retry-policy.test.ts +++ b/packages/spec/src/shared/retry-policy.test.ts @@ -12,11 +12,13 @@ * * ## Why these assertions run at RUNTIME * - * A compile-time pin cannot fail in this package (#4642): `tsconfig.json` - * excludes `**\/*.test.ts` so `pnpm typecheck` never compiles this file, and - * vitest transpiles without typechecking — a conditional-type assertion here - * would be inert. Worse, `keyof typeof import(...)` enumerates only VALUE - * exports, so a bare type name cannot be asserted that way at all. These are + * A compile-time pin could not fail in this package when this was written + * (#4642): `tsconfig.json` excluded `**\/*.test.ts` so `pnpm typecheck` never + * compiled this file, and vitest transpiles without typechecking — a + * conditional-type assertion here was inert. #5286 fixed the tsconfig half. + * The second reason stands unchanged and is why these assertions do not move: + * `keyof typeof import(...)` enumerates only VALUE exports, so a bare type name + * cannot be asserted that way at all. These are * reference-identity and shape checks on the loaded module namespaces, which * actually execute. * diff --git a/packages/spec/src/studio/action-location-retirement.test.ts b/packages/spec/src/studio/action-location-retirement.test.ts index 1177055f3e..da2b675072 100644 --- a/packages/spec/src/studio/action-location-retirement.test.ts +++ b/packages/spec/src/studio/action-location-retirement.test.ts @@ -26,7 +26,7 @@ import { describe, it, expect } from 'vitest'; // so renaming it would replay the objectui#3235 downstream breakage. // // #4642 established that a compile-time conditional-type pin in this package -// is a no-op (tsconfig excludes `**/*.test.ts`; vitest never enables +// was a no-op until #5286 (tsconfig excluded `**/*.test.ts`; vitest never enables // `typecheck`), so the load-bearing pin is the compiler-API test below, with // anti-vacuity guards; sabotage-verified in the PR (re-exporting the ui enum // from ./studio under the bare name — green to the dual-source gate, a lie to diff --git a/packages/spec/src/system/email-config.test.ts b/packages/spec/src/system/email-config.test.ts index b8fe593769..69e89b4f85 100644 --- a/packages/spec/src/system/email-config.test.ts +++ b/packages/spec/src/system/email-config.test.ts @@ -10,12 +10,13 @@ // provider the runtime had been serving since. Declared ≠ implemented, with // the spec on the *lagging* side. // -// These assertions are RUNTIME `safeParse` checks on purpose. `packages/spec` -// excludes `**/*.test.ts` from its `tsconfig.json`, so `tsc --noEmit` never -// reads this file (#5286) and any `Assert< Equal< … > >`-style type-level -// witness written here would be a phantom check that passes because nothing -// type-checks it. What the enum *accepts* is observable at runtime; that is -// what we pin. +// These assertions are RUNTIME `safeParse` checks on purpose. When they were +// written `packages/spec` excluded `**/*.test.ts` from its `tsconfig.json`, so +// `tsc --noEmit` never read this file and any `Assert< Equal< … > >`-style +// type-level witness here would have been a phantom check that passes because +// nothing type-checks it. #5286 closed that (the sibling `tsconfig.test.json` +// compiles the test layer), but these stay runtime checks on their own merit: +// what the enum *accepts* is observable at runtime, and that is what we pin. // // The set equality below is deliberately a literal. It is the spec-side half // of a two-sided pin: `spec-provider-parity.contract.test.ts` in diff --git a/packages/spec/src/system/environment-artifact.test.ts b/packages/spec/src/system/environment-artifact.test.ts index 663b1889c5..adc73ac53b 100644 --- a/packages/spec/src/system/environment-artifact.test.ts +++ b/packages/spec/src/system/environment-artifact.test.ts @@ -34,7 +34,7 @@ import { // fresh declaration on ./cloud is the forbidden route (S2 sabotage below). // // #4642 established that a compile-time conditional-type pin in this package -// is a no-op (tsconfig excludes `**/*.test.ts`; vitest never enables +// was a no-op until #5286 (tsconfig excluded `**/*.test.ts`; vitest never enables // `typecheck`), so the load-bearing pin is the compiler-API test below, with // anti-vacuity guards; sabotage-verified in the PR. diff --git a/packages/spec/src/system/http-server.test.ts b/packages/spec/src/system/http-server.test.ts index ca71b8454e..1aa84970b9 100644 --- a/packages/spec/src/system/http-server.test.ts +++ b/packages/spec/src/system/http-server.test.ts @@ -33,8 +33,10 @@ describe('HttpServerConfig retirement (#4938)', () => { // runtime `in` check is a real witness for them — reverse-verified by pasting // the removed limb back, which turns these red plus the barrel assertion // below. `HttpServerConfigInput` is type-only and cannot be seen from here at - // all: a `@ts-expect-error` pin would be a PHANTOM check, because this - // package's tsconfig excludes `**/*.test.ts` from `tsc --noEmit`. Its witness + // all: when this was written a `@ts-expect-error` pin would have been a + // PHANTOM check, because the package tsconfig excluded `**/*.test.ts` from + // `tsc --noEmit` — the hole #5286 closed with a sibling `tsconfig.test.json`, + // so a type-level pin here would evaluate today. Its longer-lived witness // is `api-surface.json`, which lost all three entries in this change and is // ratcheted by `check:api-surface` against the built `dist/*.d.ts`. it.each([ diff --git a/packages/spec/src/system/notification.test.ts b/packages/spec/src/system/notification.test.ts index 8fafd68d41..c0bf0c3ea2 100644 --- a/packages/spec/src/system/notification.test.ts +++ b/packages/spec/src/system/notification.test.ts @@ -41,9 +41,12 @@ describe('NotificationChannelSchema', () => { // #4610 in this very file, and as #4767/#4783). // // WHY THIS PIN IS A COMPILER-API TEST. #4642 established that a conditional -// type over `typeof import(...)` in this package is a NO-OP — `tsconfig.json` -// excludes `**/*.test.ts` and vitest never enables `typecheck`, so nothing ever -// evaluates it. The `NotificationConfig` pin #4610 left here was exactly that +// type over `typeof import(...)` in this package was a NO-OP until #5286 — `tsconfig.json` +// excluded `**/*.test.ts` and vitest never enables `typecheck`, so nothing ever +// evaluated it. (#5286's sibling `tsconfig.test.json` now does; the +// compiler-API test stays load-bearing because it resolves EVERY public entry, +// which a same-module conditional cannot.) The `NotificationConfig` pin #4610 +// left here was exactly that // shape; it is folded into the load-bearing test below rather than left as a // gate that cannot fail. Sabotage-verified in the PR: S1 re-declares a removed // const in notification.zod.ts, S2 re-exports it from another entry under the diff --git a/packages/spec/src/system/translation-typegen.test.ts b/packages/spec/src/system/translation-typegen.test.ts index cc28722a62..24e3fbc93c 100644 --- a/packages/spec/src/system/translation-typegen.test.ts +++ b/packages/spec/src/system/translation-typegen.test.ts @@ -99,9 +99,13 @@ describe('StrictObjectTranslation', () => { it('should report TS error when a field is missing', () => { type T = StrictObjectTranslation; - // @ts-expect-error — missing 'email' field const _invalid: T = { label: 'Test', + // The directive sits on the OFFENDING property, not on the declaration: + // TS2741 ("Property 'email' is missing") is reported at `fields`, so a + // directive three lines up suppressed nothing and was itself unused + // (TS2578). Invisible until #5286 put this file in front of tsc. + // @ts-expect-error — missing 'email' field fields: { name: { label: 'Name' }, }, @@ -125,13 +129,15 @@ describe('StrictObjectTranslation', () => { it('should report TS error when an option is missing', () => { type T = StrictObjectTranslation; - // @ts-expect-error — missing 'closed' option const _invalid: T = { label: 'Test', fields: { title: { label: 'Title' }, status: { label: 'Status', + // Same mispositioning as above: the missing-option error lands on + // `options`, not on the declaration line (#5286). + // @ts-expect-error — missing 'closed' option options: { open: 'Open' }, }, priority: { diff --git a/packages/spec/src/ui/app.test.ts b/packages/spec/src/ui/app.test.ts index b9712ee493..5f46325bf2 100644 --- a/packages/spec/src/ui/app.test.ts +++ b/packages/spec/src/ui/app.test.ts @@ -1536,9 +1536,10 @@ describe('unknown keys are rejected, not stripped (#4001 PR B)', () => { // The type-level half, and the only way to measure it that is not a no-op. // // `NavigationArea` is a TYPE — erased before any runtime assertion can see - // it — so an `Assert< Equal< … > >` pin written in this file would never - // run: `packages/spec/tsconfig.json` excludes `**/*.test.ts` and vitest does - // not type-check (#4642). Every assertion above would stay green if the two + // it — and when this was written an `Assert< Equal< … > >` pin in this file + // would never have run: `packages/spec/tsconfig.json` excluded + // `**/*.test.ts` and vitest does not type-check (#4642; the tsconfig half is + // closed by #5286). Every assertion above would stay green if the two // keys were re-added to the schema's TS shape while the parse kept // rejecting them. This resolves the exported type through the TypeScript // compiler API and reads its members — the same symbol-identity measurement diff --git a/packages/spec/src/ui/dashboard.test.ts b/packages/spec/src/ui/dashboard.test.ts index f2908a9245..fdf056adb2 100644 --- a/packages/spec/src/ui/dashboard.test.ts +++ b/packages/spec/src/ui/dashboard.test.ts @@ -213,9 +213,9 @@ describe('Dashboard presentation sub-schemas', () => { // ============================================================================ // // RUNTIME assertions, deliberately. #4642 established that a compile-time pin in -// `packages/spec` is a no-op: `tsconfig.json` excludes `**/*.test.ts` and +// `packages/spec` was a no-op until #5286: `tsconfig.json` excluded `**/*.test.ts` and // `vitest.config.ts` never enables `typecheck`, so an `Assert< Equal< … > >` -// here would be dead text. The tombstone's `tsc` channel is proved by the build +// here was dead text until #5286. The tombstone's `tsc` channel is proved by the build // of the packages that author dashboards, not by this file. // // The pair below is the whole contract of this retirement: the widget embed @@ -283,7 +283,7 @@ describe('[#4876] DashboardWidgetSchema — retired `responsive`', () => { // ============================================================================ // // RUNTIME assertions for the same reason the #4876 block above gives: a -// compile-time pin in `packages/spec` is dead text (#4642). The tombstone's +// compile-time pin in `packages/spec` was dead text until #5286 (#4642). The tombstone's // `tsc` channel is proved by the build of the packages that author dashboards. // // Two affordances, one block, because they were retired as one change: diff --git a/packages/spec/src/ui/interaction-config-retirement.test.ts b/packages/spec/src/ui/interaction-config-retirement.test.ts index 1714ae218a..cead4a9e17 100644 --- a/packages/spec/src/ui/interaction-config-retirement.test.ts +++ b/packages/spec/src/ui/interaction-config-retirement.test.ts @@ -49,7 +49,7 @@ import { describe, it, expect } from 'vitest'; // // Form follows #4834 / PR #5300: resolved symbol identity over every public // entry in `package.json`'s exports map. #4642 established that a compile-time -// conditional-type pin in this package is a no-op (tsconfig excludes +// conditional-type pin in this package was a no-op until #5286 (tsconfig excluded // `**/*.test.ts`; vitest never enables `typecheck`), so the compiler-API walk // with anti-vacuity guards is the load-bearing instrument. describe('[#4988] ui/ interaction config family retirement', () => { diff --git a/packages/spec/src/ui/notification-embed-retirement.test.ts b/packages/spec/src/ui/notification-embed-retirement.test.ts index cd68e698d7..e8dca90ad7 100644 --- a/packages/spec/src/ui/notification-embed-retirement.test.ts +++ b/packages/spec/src/ui/notification-embed-retirement.test.ts @@ -32,9 +32,9 @@ import { describe, it, expect } from 'vitest'; // form sharing, and "the names are gone" alone cannot tell the two apart. // // Why THIS pin and not a type-level one: #4642 established that a compile-time -// conditional-type assertion in this package is a no-op (the package tsconfig -// excludes `**/*.test.ts`, and vitest never enables `typecheck`), so an -// `Assert< Equal< … > >` here would be decoration. The load-bearing pin is the +// conditional-type assertion in this package was a no-op until #5286 (the package tsconfig +// excluded `**/*.test.ts`, and vitest never enables `typecheck`), so an +// `Assert< Equal< … > >` here was decoration until #5286. The load-bearing pin is the // TypeScript compiler-API program below, which resolves the REAL export surface // of every public entry from `package.json`'s exports map and asserts each // retired name has zero holders — by symbol identity, not by grepping text. diff --git a/packages/spec/src/ui/view.test.ts b/packages/spec/src/ui/view.test.ts index 4f27c27f55..90e8f4593a 100644 --- a/packages/spec/src/ui/view.test.ts +++ b/packages/spec/src/ui/view.test.ts @@ -2655,12 +2655,13 @@ describe('HttpMethodSchema/HttpRequestSchema backward compat', () => { // ─── [#4688] Dual-source regression pin ────────────────────────────── // // RUNTIME assertions, deliberately. #4642 established that a compile-time pin in -// `packages/spec` is a no-op: `tsconfig.json` excludes `**/*.test.ts` and -// `vitest.config.ts` never enables `typecheck`, so neither path type-checks a -// test file. A conditional-type `Assert< Equal< … > >` here would be dead text — -// and so, for the same reason, is the bare `type HttpRequest` import at the top -// of this file: vitest's transform erases it, so it proves nothing about the -// export still existing. The third test below is what actually proves that. +// `packages/spec` was a no-op until #5286: `tsconfig.json` excluded `**/*.test.ts` and +// `vitest.config.ts` never enables `typecheck`, so neither path type-checked a +// test file. A conditional-type `Assert< Equal< … > >` here was dead text until +// #5286. One half of that argument survives #5286 untouched and is why these +// stay runtime assertions: the bare `type HttpRequest` import at the top of this +// file proves nothing about the export still existing, because vitest's +// transform erases it. The third test below is what actually proves that. // // What these defend: `HttpRequest` naming ONE declaration across both published // entries. `HttpRequestSchema` was never duplicated — `./ui` imports and @@ -2765,7 +2766,7 @@ describe('[#4688] HttpRequest is single-source across ./shared and ./ui', () => // // Same reasoning as #4688 on the mechanism: `HttpMethod` is a TYPE, erased // before any runtime assertion can see it, and #4642 established that a -// compile-time pin in this package is a no-op (`tsconfig.json` excludes +// compile-time pin in this package was a no-op until #5286 (`tsconfig.json` excluded // `**/*.test.ts`; vitest never enables `typecheck`). The compiler-API test is // therefore the load-bearing one; the runtime tests below it guard the value // ranges the whole argument rests on. diff --git a/packages/spec/test-typecheck-debt.json b/packages/spec/test-typecheck-debt.json new file mode 100644 index 0000000000..7f606463d4 --- /dev/null +++ b/packages/spec/test-typecheck-debt.json @@ -0,0 +1,84 @@ +{ + "_comment": "Per-file tsc error debt of the @objectstack/spec TEST layer (#5286). `tsconfig.test.json` compiles `src/**/*.test.ts` — which `tsconfig.json` excludes and therefore no gate ever read — and every file below still carries errors from before that gate existed, almost all of them fixture literals annotated with a schema OUTPUT type (`z.infer`) while holding an authored INPUT literal. EXACT ratchet, judged by re-running tsc: a file that gains errors is red, a file that loses them is red until its number is re-recorded, a file that reaches zero is red until its entry is deleted, and a file NOT listed here may have no errors at all. Regenerate with: pnpm --filter @objectstack/spec gen:test-typecheck-debt", + "entries": { + "src/ai/agent.test.ts": 11, + "src/ai/conversation.test.ts": 13, + "src/ai/model-registry.test.ts": 24, + "src/ai/skill.test.ts": 1, + "src/api/documentation.test.ts": 1, + "src/api/errors.test.ts": 1, + "src/api/metadata.test.ts": 1, + "src/api/odata.test.ts": 1, + "src/api/package-api.test.ts": 4, + "src/api/rest-server.test.ts": 3, + "src/api/router.test.ts": 9, + "src/automation/control-flow.test.ts": 5, + "src/automation/schemaless-node-config.test.ts": 1, + "src/automation/webhook.test.ts": 1, + "src/compose-stacks-key-loss.test.ts": 7, + "src/compose-stacks.test.ts": 69, + "src/contracts/ai-service.test.ts": 1, + "src/contracts/analytics-service.test.ts": 1, + "src/contracts/app-lifecycle-service.test.ts": 1, + "src/contracts/automation-service.test.ts": 1, + "src/contracts/core-service-contracts.test.ts": 15, + "src/contracts/data-engine.test.ts": 1, + "src/contracts/export-service.test.ts": 3, + "src/contracts/http-server.test.ts": 3, + "src/contracts/logger.test.ts": 2, + "src/contracts/metadata-service.test.ts": 2, + "src/contracts/package-service.test.ts": 11, + "src/contracts/plugin-lifecycle-events.test.ts": 2, + "src/contracts/security-service.test.ts": 1, + "src/contracts/seed-loader-service.test.ts": 4, + "src/contracts/service-registry.test.ts": 7, + "src/contracts/storage-service.test.ts": 1, + "src/data/data-engine.test.ts": 6, + "src/data/datasource.test.ts": 1, + "src/data/display-name.test.ts": 18, + "src/data/driver-nosql.test.ts": 2, + "src/data/driver-sql.test.ts": 1, + "src/data/driver.test.ts": 11, + "src/data/driver/memory.test.ts": 1, + "src/data/field.test.ts": 19, + "src/data/mapping.test.ts": 4, + "src/data/object-strictness-batch20.test.ts": 2, + "src/data/query.test.ts": 25, + "src/data/seed.test.ts": 1, + "src/identity/identity.test.ts": 1, + "src/identity/position.test.ts": 27, + "src/identity/scim.test.ts": 8, + "src/integration/connector.test.ts": 11, + "src/kernel/activation-events-retirement.test.ts": 1, + "src/kernel/cluster.test.ts": 2, + "src/kernel/events.test.ts": 14, + "src/kernel/manifest.test.ts": 16, + "src/kernel/metadata-plugin.test.ts": 1, + "src/kernel/public-auth-features.test.ts": 6, + "src/security/permission.test.ts": 28, + "src/security/sharing.test.ts": 1, + "src/shared/metadata-collection.test.ts": 1, + "src/stack.test.ts": 33, + "src/system/app-install.test.ts": 1, + "src/system/collaboration.test.ts": 6, + "src/system/deploy-bundle.test.ts": 3, + "src/system/disaster-recovery.test.ts": 3, + "src/system/i18n-resolver.test.ts": 11, + "src/system/job.test.ts": 8, + "src/system/logging.test.ts": 15, + "src/system/metrics.test.ts": 15, + "src/system/object-storage.test.ts": 14, + "src/system/tenant.test.ts": 3, + "src/system/tracing.test.ts": 14, + "src/system/worker.test.ts": 11, + "src/ui/action.test.ts": 20, + "src/ui/app.test.ts": 19, + "src/ui/chart.test.ts": 5, + "src/ui/i18n.test.ts": 1, + "src/ui/page.test.ts": 1, + "src/ui/report.test.ts": 3, + "src/ui/theme.test.ts": 6, + "src/ui/view.test.ts": 79, + "src/ui/widget.test.ts": 4 + } +} diff --git a/packages/spec/tsconfig.test.json b/packages/spec/tsconfig.test.json new file mode 100644 index 0000000000..f15cc97ef1 --- /dev/null +++ b/packages/spec/tsconfig.test.json @@ -0,0 +1,35 @@ +// The TEST-layer type-check program (#5286). `tsconfig.json` above stays as it +// is: it is the BUILD config, and its `**/*.test.ts` exclusion has a reason — +// ci.yml gates that no test file reaches the published artifact. This sibling +// puts the excluded layer back in front of tsc, and `package.json`'s +// `typecheck` script names it (`check:test-typecheck --project`), because a +// config no script invokes is exactly the phantom this whole change is about. +// +// What differs from the build config, and what deliberately does NOT: +// - module semantics ONLY. The tests are written and executed as ESM by +// vitest (esbuild/vite), while `spec` has no `"type": "module"`, so the +// build config's NodeNext compiles them as CJS and reports 108 errors about +// the CHECK rather than the code (TS2835 dynamic-import extensions, TS1470 +// `import.meta`, TS2307, TS2550). Matching vitest is fidelity. +// - STRICTNESS IS UNTOUCHED. `strict`, `noUnusedLocals`, `noImplicitReturns` +// and the rest are inherited from the root config. Nothing about this file +// may loosen a type rule; if a test does not compile, that is the finding. +// +// `include` deliberately stops at `src`, matching the build config's root. +// `scripts/` holds nine more test files vitest runs and is in no tsconfig at +// all — a second, differently-shaped hole (measured at 16 files / 33 errors, +// mostly config-tier TS5097/TS2593 plus a real TS2339 pile in build-schemas.ts) +// that wants its own change rather than a rider on this one. None of those +// files carries a `@ts-expect-error`, so no pin is hiding there. +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "noEmit": true, + "module": "esnext", + "moduleResolution": "bundler", + "lib": ["ES2022", "DOM", "DOM.Iterable"], + "types": ["node"] + }, + "include": ["src/**/*"], + "exclude": ["node_modules", "dist"] +} diff --git a/scripts/check-type-check-coverage.mjs b/scripts/check-type-check-coverage.mjs index a9fa65470c..a72f3e68b7 100644 --- a/scripts/check-type-check-coverage.mjs +++ b/scripts/check-type-check-coverage.mjs @@ -56,6 +56,29 @@ // #4311 is actually about. This invariant is why the ledger is // two ledgers: DEBT is "src does not check", TEST_DEBT is "src // checks, tests are hidden", and they are independent. +// +// Read across ALL of a package's tsconfigs, not just +// `tsconfig.json` (#5286). The build config has a reason to +// exclude tests -- ci.yml gates that no test file reaches the +// published artifact -- so the supported repair is a SIBLING +// `tsconfig.test.json` wired into the `typecheck` script. Judging +// only `tsconfig.json` would keep calling such a package hidden +// while tsc reads every one of its tests. The sibling must be +// NAMED in the typecheck script chain: a config no script invokes +// reads as coverage and delivers none, which is this gate's own +// subject matter. +// PINS_CHECKED +// a test file containing a `@ts-expect-error` directive sits +// inside a tsc program, or is listed in PHANTOM_PIN_DEBT below. +// `@ts-expect-error` is the retirement channel the +// spec-property-retirement playbook leans on ("tsc is the best +// sweeper"): the directive is supposed to go red the day the +// removed key comes back. In an unchecked file it evaluates +// NEVER -- deleting the directive line leaves the suite just as +// green, which is a phantom check wearing a pin's clothes +// (#5286). Independent of TESTS_COVERED: a file can sit outside +// `include` without any exclusion naming it, which is how +// `packages/metadata-core/test/` hid. // RUNNABLE turbo.json declares the `typecheck` task, the root `typecheck` // script aggregates it (`turbo run typecheck`, the build/test // convention), and lint.yml invokes it -- a script CI never @@ -98,6 +121,12 @@ const TRACKING_ISSUE = 'https://github.com/objectstack-ai/objectstack/issues/431 // "does this config steer tsc away from the test layer", not "which exact glob". const TEST_GLOB = /\*\.(test|spec)\.tsx?$/; const TEST_FILE = /\.(test|spec)\.tsx?$/; +// A `@ts-expect-error` in DIRECTIVE position -- first thing on its own comment +// line, where the compiler reads it. Prose that merely mentions the directive +// (several files in this repo explain why they do NOT use one) must not count, +// or PINS_CHECKED would fire on documentation. +const PIN_DIRECTIVE = /^[ \t]*(?:\/\/|\/\*|\*)[ \t]*@ts-expect-error\b/m; +const PIN_ISSUE = 'https://github.com/objectstack-ai/objectstack/issues/5286'; // Package name -> { errors, note? }. `errors` is the raw `tsc --noEmit` count // measured per package on main @ b07d829 (2026-07-31), re-measured after the @@ -199,12 +228,16 @@ const EXEMPT = { // declare `typecheck`. What they hide is the test layer, which is where #4311 // found the defects (a passing vitest run proves the code executes, not that // the call shapes match). Sorted by what each is hiding, worst first. +// `@objectstack/spec` graduated in #5286: `tsconfig.test.json` (a sibling of +// the build config, named by the `typecheck` script) compiles its 295 test +// files, so nothing is hidden any more and TESTS_COVERED no longer wants an +// entry here. The residue that lifting the exclusion surfaced did not vanish +// with the entry -- it moved to `packages/spec/test-typecheck-debt.json`, a +// PER-FILE exact ratchet re-measured by tsc on every run, which is strictly +// stronger than the frozen package-level number this ledger could hold. The +// number that used to sit here (272 files / 902 errors) was also stale by 23 +// files, which is the other argument for a measurement the gate derives. const TEST_DEBT = { - '@objectstack/spec': { - tests: 272, - errors: 902, - note: 'TS6133 x208, TS2739 x193, TS2741 x146, TS2322 x96 -- overwhelmingly incomplete object literals in test fixtures against the schemas spec itself defines.', - }, '@objectstack/plugin-approvals': { tests: 13, errors: 467, @@ -231,6 +264,25 @@ const TEST_DEBT = { '@objectstack/connector-rest': { tests: 3, errors: 1, note: 'TS6133 x1.' }, }; +// Repo-relative path -> why this test file's `@ts-expect-error` directives are +// still phantom. PINS_CHECKED's escape hatch, and the narrowest of the three +// ledgers on purpose: an unchecked pin is not "debt we measured", it is a +// retirement guard that reads as enforced and enforces nothing. A directive in +// one of these files can be DELETED with no gate noticing -- which is how +// #5286's 17 spec directives were found. +// +// Shrink-only, and closed: a file that starts carrying a pin while unchecked +// fails PINS_CHECKED rather than joining this list. The repair is the same one +// spec took -- put the file in a tsc program (drop the exclusion, widen +// `include`, or add a sibling `tsconfig.test.json` the typecheck script names) +// -- and then delete the entry, which RECONCILED forces anyway. +const PHANTOM_PIN_DEBT = { + 'packages/client/src/client.test.ts': + 'tsconfig.json excludes `**/*.test.ts` and the package has no sibling test config; also in TEST_DEBT (15 files / 19 errors). Onboarding it is #5449, not #5286 -- the two directives here pin retired client options.', + 'packages/metadata-core/test/types.test.ts': + 'Outside the program for a different reason, and one no exclusion names: `include` is `["src/**/*"]` while this file lives in a sibling `test/` tree, so TESTS_COVERED never saw it either (its testFiles count is 0). Repair is to widen `include` or add a test config; tracked by #5476, not by #5286 (which scoped itself to packages/spec).', +}; + /** * The `packages:` globs from pnpm-workspace.yaml. Blank lines and comments are * skipped rather than treated as the end of the list: stopping early would @@ -256,43 +308,112 @@ function workspaceGlobs() { } /** - * Does this package's tsconfig `exclude` its own test files, and how many are - * there to hide? Read with a tolerant parse -- these configs carry `//` - * comments, and a parse failure must not silently read as "excludes nothing" - * (that would turn TESTS_COVERED into a gate that passes on unparseable input). + * One `tsconfig*.json` of a package, read with a tolerant parse -- these configs + * carry `//` comments, and a parse failure must not silently read as "excludes + * nothing" (that would turn TESTS_COVERED into a gate that passes on + * unparseable input). * - * @returns {{excludesTests: boolean, testFiles: number}} + * `roots` come from the `include` glob prefixes (`src/**\/*` -> `src`); no + * `include` at all means tsc walks the whole package directory, which is the + * empty root. + * + * @returns {{file: string, roots: string[], excludesTests: boolean}} */ -function testCoverage(dir) { - const tsconfigPath = join(ROOT, dir, 'tsconfig.json'); - let excludesTests = false; - let parsedInclude = null; - if (existsSync(tsconfigPath)) { - const raw = readFileSync(tsconfigPath, 'utf8').replace(/^\s*\/\/.*$/gm, ''); - let parsed; - try { - parsed = JSON.parse(raw); - } catch (cause) { - throw new Error(`${dir}/tsconfig.json is not parseable, so its test coverage cannot be judged`, { cause }); +function readTsconfig(dir, file) { + const raw = readFileSync(join(ROOT, dir, file), 'utf8').replace(/^\s*\/\/.*$/gm, ''); + let parsed; + try { + parsed = JSON.parse(raw); + } catch (cause) { + throw new Error(`${dir}/${file} is not parseable, so its test coverage cannot be judged`, { cause }); + } + const include = parsed.include ?? null; + const roots = Array.isArray(include) && include.length > 0 + ? [...new Set(include.map((g) => g.split('*')[0].replace(/\/$/, '')).filter((p) => !p.includes('..')))] + : ['']; + return { + file, + roots, + excludesTests: (parsed.exclude ?? []).some((pattern) => TEST_GLOB.test(pattern)), + }; +} + +/** Is `rel` (posix, relative to the package) inside this config's program? */ +function configCovers(config, rel) { + if (config.excludesTests && TEST_FILE.test(rel)) return false; + return config.roots.some((root) => root === '' || rel === root || rel.startsWith(`${root}/`)); +} + +/** + * Which tsconfig files does the `typecheck` script actually put in front of + * tsc? Expanded through same-package `pnpm