From e68bf475f9d0b0c838577821cec042f877664cd6 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 13:14:29 +0000 Subject: [PATCH 1/3] fix(spec): compile the test layer so `@ts-expect-error` retirement pins stop being phantom checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `packages/spec/tsconfig.json` excludes `**/*.test.ts` and the package's `typecheck` script is a bare `tsc --noEmit` reading that same config, so no gate anywhere read a spec test file with a type checker. Seventeen `@ts-expect-error` retirement pins across five files evaluated never: deleting a directive line left every gate green. - `packages/spec/tsconfig.test.json`: a sibling of the build config (which keeps its exclusion — ci.yml gates that no test file reaches the published artifact) with vitest's module semantics (`module: esnext`, `moduleResolution: bundler`, ES2022 lib). Strictness flags are inherited, untouched. - 39 errors in the five pin files fixed outright, plus the real defects the compile surfaced: two missing `z` imports (TS2503), an `as const[]` typo (TS2304), a `session.tenantId` fixture key removed in v11 (#3290), and two `@ts-expect-error` directives sitting three lines above the error they claim to suppress. - Residual test-layer debt (79 files / 691 errors, mostly fixture literals typed with `z.infer` while holding `z.input` values) is held per file in `test-typecheck-debt.json`, an EXACT ratchet re-measured by tsc on every run. - `check-type-check-coverage.mjs`: TESTS_COVERED now reads every tsconfig a package's typecheck chain names, so the sibling-config repair graduates a package instead of leaving it in TEST_DEBT forever; spec's stale entry (272 files / 902 errors; actual 295 / 842) is deleted accordingly. New repo-wide PINS_CHECKED invariant: a `@ts-expect-error` outside every tsc program fails, with a closed shrink-only PHANTOM_PIN_DEBT baseline. Fixes #5286 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_018fxLGQdatPbBUvCgiVxg6D --- AGENTS.md | 18 +- packages/spec/package.json | 4 +- .../spec/scripts/check-test-typecheck.mts | 306 ++++++++++++++++ packages/spec/src/api/endpoint.test.ts | 4 + .../spec/src/api/registry-retirement.test.ts | 4 +- packages/spec/src/api/rest-server.test.ts | 4 +- packages/spec/src/api/router.test.ts | 4 + .../spec/src/automation/state-machine.test.ts | 2 +- .../src/automation/sync-retirement.test.ts | 2 +- packages/spec/src/cloud/tenant.test.ts | 2 +- packages/spec/src/data/driver.test.ts | 4 +- .../src/data/filter-array-declaration.test.ts | 44 +-- packages/spec/src/data/hook.test.ts | 17 +- packages/spec/src/data/object.test.ts | 38 +- packages/spec/src/identity/identity.test.ts | 14 +- .../spec/src/integration/connector.test.ts | 8 +- .../activation-events-retirement.test.ts | 4 +- .../spec/src/kernel/package-artifact.test.ts | 5 +- .../package-dependency-dual-source.test.ts | 4 +- .../kernel/plugin-runtime-retirement.test.ts | 20 +- packages/spec/src/shared/retry-policy.test.ts | 12 +- .../studio/action-location-retirement.test.ts | 2 +- packages/spec/src/system/email-config.test.ts | 13 +- .../src/system/environment-artifact.test.ts | 2 +- packages/spec/src/system/http-server.test.ts | 6 +- packages/spec/src/system/notification.test.ts | 9 +- .../src/system/translation-typegen.test.ts | 10 +- packages/spec/src/ui/app.test.ts | 7 +- packages/spec/src/ui/dashboard.test.ts | 6 +- .../ui/interaction-config-retirement.test.ts | 2 +- .../ui/notification-embed-retirement.test.ts | 6 +- packages/spec/src/ui/view.test.ts | 15 +- packages/spec/test-typecheck-debt.json | 84 +++++ packages/spec/tsconfig.test.json | 12 + scripts/check-type-check-coverage.mjs | 343 +++++++++++++++--- 35 files changed, 884 insertions(+), 153 deletions(-) create mode 100644 packages/spec/scripts/check-test-typecheck.mts create mode 100644 packages/spec/test-typecheck-debt.json create mode 100644 packages/spec/tsconfig.test.json 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-test-typecheck.mts b/packages/spec/scripts/check-test-typecheck.mts new file mode 100644 index 0000000000..3ddec8ae4a --- /dev/null +++ b/packages/spec/scripts/check-test-typecheck.mts @@ -0,0 +1,306 @@ +#!/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 SELF = 'scripts/check-test-typecheck.mts'; +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..ac88654c2a --- /dev/null +++ b/packages/spec/tsconfig.test.json @@ -0,0 +1,12 @@ +{ + "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..95435aeca2 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; measured separately from #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