diff --git a/packages/spec/package.json b/packages/spec/package.json index d4bc41a560..ceb65316de 100644 --- a/packages/spec/package.json +++ b/packages/spec/package.json @@ -220,7 +220,8 @@ "check:skill-examples": "tsx scripts/check-skill-examples.ts --self-test && tsx scripts/check-skill-examples.ts", "check:test-typecheck": "tsx ../../scripts/check-test-typecheck.mts --self-test && tsx ../../scripts/check-test-typecheck.mts --package packages/spec --project tsconfig.test.json", "gen:test-typecheck-debt": "tsx ../../scripts/check-test-typecheck.mts --update --package packages/spec --project tsconfig.test.json", - "typecheck": "tsc --noEmit && pnpm check:test-typecheck" + "check:scripts-typecheck": "tsc --noEmit -p tsconfig.scripts.json", + "typecheck": "tsc --noEmit && pnpm check:scripts-typecheck && pnpm check:test-typecheck" }, "keywords": [ "objectstack", diff --git a/packages/spec/scripts/authorable-defaults.test.ts b/packages/spec/scripts/authorable-defaults.test.ts index 486fd1b834..e5e44720b2 100644 --- a/packages/spec/scripts/authorable-defaults.test.ts +++ b/packages/spec/scripts/authorable-defaults.test.ts @@ -87,10 +87,15 @@ describe('collectAuthorableDefaults — what the fingerprint reads', () => { // loudly, so it is a different and self-announcing class. If this ever // fails, someone has widened the ratchet into direction A — which was // considered and declined, not overlooked. - const loose = [ + // Typed as the collector's own parameter, not `as const`: a `readonly` + // tuple is not an `Iterable<[string, unknown]>`, so these two fixtures did + // not type-check at all — invisible until #5475 put `scripts/` in a tsc + // program. Naming the producer's type also keeps the fixture honest if that + // signature ever changes. + const loose: Array<[string, unknown]> = [ ['system/Job', { properties: { maxRetries: { type: 'integer', minimum: 0, default: 0 } } }], - ] as const; - const tightened = [ + ]; + const tightened: Array<[string, unknown]> = [ [ 'system/Job', { @@ -105,7 +110,7 @@ describe('collectAuthorableDefaults — what the fingerprint reads', () => { }, }, ], - ] as const; + ]; expect([...collectAuthorableDefaults(tightened)]).toEqual([...collectAuthorableDefaults(loose)]); }); diff --git a/packages/spec/scripts/build-docs.ts b/packages/spec/scripts/build-docs.ts index 91de2f9910..a6a8f1d3e3 100644 --- a/packages/spec/scripts/build-docs.ts +++ b/packages/spec/scripts/build-docs.ts @@ -300,7 +300,11 @@ function sourcePathToDocsRoute(target: string): string | null { // because a helper happened to sit at the top of the file, and `check:docs` // could not see it (the artifact reproduced the wrong block faithfully). -function generateMarkdown(schemaName: string, schema: any, category: string, zodFile: string) { +// `_zodFile` is passed by the caller and deliberately unread here: the file slug +// is a page-level fact, and every use of it (title, source link, card) lives in +// `generateZodFileMarkdown` around this call. Underscored rather than dropped so +// this touches one line of a renderer PR #6377 is editing (#5475). +function generateMarkdown(schemaName: string, schema: any, category: string, _zodFile: string) { const defs = schema.definitions || schema.$defs || {}; let mainDef = defs[schemaName]; @@ -721,8 +725,6 @@ Object.keys(CATEGORIES).forEach(category => { managedCount++; }); -const generatedFiles: string[] = []; - // 2. Generate Files // Clear DOCS_ROOT first to remove old flattened files if (fs.existsSync(DOCS_ROOT)) { diff --git a/packages/spec/scripts/build-openapi.ts b/packages/spec/scripts/build-openapi.ts index 29e03fdcd7..fc7edfea4a 100644 --- a/packages/spec/scripts/build-openapi.ts +++ b/packages/spec/scripts/build-openapi.ts @@ -6,7 +6,12 @@ import { z } from 'zod'; // Dynamic imports from spec source import * as API from '../src/api'; -import * as Data from '../src/data'; +// `import * as Data from '../src/data'` used to sit here, bound and never read. +// It was already a no-op at runtime — TS import elision drops an unused +// namespace import before tsx ever evaluates it — so this removes a name, not a +// side effect; `json-schema/openapi.json` is byte-identical across the change +// (#5475). Restoring the module evaluation, had it been load-bearing, would +// have meant a bare `import '../src/data';`, which is a different statement. import { assertRefsResolve, assertNoDegradedSchemas } from './lib/openapi-self-consistency'; // The name this generator writes is DECLARED next to `build-schemas.ts`'s clean // step, which shares this directory and must not sweep it away (#5371). Imported diff --git a/packages/spec/scripts/build-schemas.ts b/packages/spec/scripts/build-schemas.ts index 7f9c6b3b54..fff00ba113 100644 --- a/packages/spec/scripts/build-schemas.ts +++ b/packages/spec/scripts/build-schemas.ts @@ -27,9 +27,7 @@ import { clearOwnedOutputs, } from './lib/json-schema-out-dir'; import { - AUTHORABLE_SURFACE_DESCRIPTION, AUTHORABLE_SURFACE_DIR_NAME, - SCHEMA_MANIFEST_DESCRIPTION, SCHEMA_MANIFEST_DIR_NAME, aggregateCategoryShards, authorableSurfaceShardTexts, @@ -38,6 +36,7 @@ import { serializeShard, writeShards, type GitRun, + type ShardArrayField, } from './lib/sharded-artifacts'; // The #4666 default-value ratchet: what an author gets when they OMIT a key. // Its own module because the fingerprint's normalisation rules — and the @@ -483,15 +482,11 @@ if (defKeyCollisions.length > 0) { // run means a code change unpublished a schema — fail loudly instead of // letting gen:docs quietly delete its reference docs (#2978). Deliberate // removals must delete the key from the manifest in the same PR. -/** - * The manifest's description — the procedure a reader who opens a shard to - * delete a line follows. Until #4725 it ended "remove a key ONLY for a - * deliberate retirement", which was the entire requirement and was checked by - * nothing; it now names the gate and the table that answer for a removal. It - * lives in scripts/lib/sharded-artifacts.ts with the writer that stamps it into - * every shard (#5837). - */ -const MANIFEST_DESCRIPTION = SCHEMA_MANIFEST_DESCRIPTION; +// The manifest's and the authorable surface's shard descriptions used to be +// re-exported through here. #5837 moved both to scripts/lib/sharded-artifacts.ts, +// beside the writer that stamps them into every shard, and nothing in this file +// has read them since — the import and the `MANIFEST_DESCRIPTION` alias were +// residue no checker could see (#5475). /** * Every def key recorded across `json-schema.manifest/`, or null when the whole @@ -1094,7 +1089,13 @@ function readSurfaceKeysAtRev( git: GitRun, rev: string, dirName: string, - field: 'keys' | 'schemas', + // `ShardArrayField`, not a re-spelled copy of it. This parameter used to read + // `'keys' | 'schemas'` — a hand-written narrowing of the exported union that + // `readShardedKeysAtRev` below actually takes. When #4666 added `'defaults'` + // to `ShardArrayField` and a call site passing it, the copy here was left + // behind and no type checker existed to say so (#5475). Harmless at runtime, + // since the value is only forwarded, but it is the drift this program is for. + field: ShardArrayField, context: string, ): { entries: string[] } | null { const read = readShardedKeysAtRev(git, rev, dirName, field); @@ -1453,12 +1454,33 @@ function assertAnchorMovesForward(git: GitRun, committedRev: string, resolvedRev } /** - * Set when THIS run resolved the baseline from git. It is the ONLY input - * `--update-base` may write the in-tree anchor from: an offline build must never - * be able to advance the anchor to its own state (#5235). The second half of that - * discipline is #5358 — no build writes it at all, only the explicit mode. + * What `resolveSurfaceBase()` resolved: the baseline itself, plus — only when + * the GIT path produced it — the anchor that path is allowed to write. + * + * `gitAnchor` is a returned field rather than the module-level assignment it + * used to be, and that is a type-checking fix, not a style one (#5475). The old + * shape declared `let gitResolvedAnchor: {...} | null = null` here and assigned + * it from INSIDE this function. TypeScript's control-flow analysis does not + * follow an assignment made in a function body, so at every top-level read below + * the variable was still narrowed to `null` — which made `if (gitResolvedAnchor)` + * a block whose body is typed `never`, i.e. the entire in-tree anchor writer + * (#5235/#5358/#5370/#5847, ~100 lines) was invisible to tsc while reading as + * ordinary checked code. Returning the value puts the assignment in the caller's + * own flow, where CFA can see it. Runtime behaviour is unchanged: the git path + * sets it, the in-tree path leaves it null, exactly as before. */ -let gitResolvedAnchor: { rev: string; keys: string[] } | null = null; +type SurfaceBaseResolution = { + rev: string; + doc: AuthorableSurface; + /** + * Set when THIS run resolved the baseline from git. It is the ONLY input + * `--update-base` may write the in-tree anchor from: an offline build must + * never be able to advance the anchor to its own state (#5235). The second + * half of that discipline is #5358 — no build writes it at all, only the + * explicit mode. + */ + gitAnchor: { rev: string; keys: string[] } | null; +}; /** * The committed authorable surface this PR started from: its content at @@ -1483,7 +1505,7 @@ let gitResolvedAnchor: { rev: string; keys: string[] } | null = null; * What is NOT offered is an env-var skip: that is precisely the bypass #4650 * closes. With no anchor of either kind this still exits 1. */ -function resolveSurfaceBase(): { rev: string; doc: AuthorableSurface } | null { +function resolveSurfaceBase(): SurfaceBaseResolution | null { const git = gitInPackage; const committed = readCommittedSurfaceBase(); @@ -1520,10 +1542,10 @@ function resolveSurfaceBase(): { rev: string; doc: AuthorableSurface } | null { return null; } const doc: AuthorableSurface = { keys: baseline.entries }; - gitResolvedAnchor = { rev, keys: doc.keys }; + const gitAnchor = { rev, keys: doc.keys }; // The environment that CAN police the in-tree anchor is the one that must. - if (committed) verifyCommittedSurfaceBase(git, tip, gitResolvedAnchor, committed.doc); - return { rev, doc }; + if (committed) verifyCommittedSurfaceBase(git, tip, gitAnchor, committed.doc); + return { rev, doc, gitAnchor }; } if (committed) { @@ -1535,6 +1557,9 @@ function resolveSurfaceBase(): { rev: string; doc: AuthorableSurface } | null { return { rev: committed.doc.baseRev, doc: { keys: committed.doc.keys }, + // Offline: this run did not resolve an anchor from git, so it has nothing + // it is entitled to write one from (#5235). + gitAnchor: null, }; } @@ -1722,11 +1747,20 @@ function checkManifestRemovals(git: GitRun, baseRev: string | null): void { * is one resolution, shared — a second `resolveSurfaceBase()` call would ask git * the same question twice and could answer it differently. */ -let resolvedSurfaceBase: { rev: string; doc: AuthorableSurface } | null = null; +let resolvedSurfaceBase: SurfaceBaseResolution | null = null; + +/** + * The git-resolved anchor of this run, hoisted out of the block below because + * the in-tree anchor writer further down is a separate top-level block. + * Assigned HERE, in the module's own control flow, which is what keeps it typed + * as the union it is declared as — see `SurfaceBaseResolution.gitAnchor`. + */ +let gitResolvedAnchor: { rev: string; keys: string[] } | null = null; { const base = resolveSurfaceBase(); resolvedSurfaceBase = base; + gitResolvedAnchor = base?.gitAnchor ?? null; // Whole defs first: check (c) below waives every baseline line under a def this // build stopped emitting, on the grounds that this gate adjudicates it. Running // it first is what makes that deferral true rather than circular. @@ -1748,9 +1782,10 @@ let resolvedSurfaceBase: { rev: string; doc: AuthorableSurface } | null = null; const violations: string[] = []; const goneDefs = new Map(); // def no longer emitted -> deleted key count for (const key of deletedKeys) { - const sep = key.indexOf(':'); - const defKey = key.slice(0, sep); - const prop = key.slice(sep + 1); + // Only the def half is read now. The leaf half fed the leaf-NAME match + // #5898 removed from route 3 (see the RETIRED_KEYS_BY_MAJOR message + // below); slicing it out survived the rewrite as a dead local (#5475). + const defKey = key.slice(0, key.indexOf(':')); if (!generatedSchemas.has(defKey)) { goneDefs.set(defKey, (goneDefs.get(defKey) ?? 0) + 1); continue; diff --git a/packages/spec/scripts/check-generated.ts b/packages/spec/scripts/check-generated.ts index 4797e0330c..009601d552 100644 --- a/packages/spec/scripts/check-generated.ts +++ b/packages/spec/scripts/check-generated.ts @@ -173,6 +173,17 @@ const NO_GENERATOR: ReadonlyArray<{ check: string; why: string }> = [ check: 'check:dual-source-exports', why: 'audits the built .d.ts for same-name exports resolving to DIFFERENT declarations across entry points — baseline is hand-ratcheted, not generated (needs a fresh `pnpm build`)', }, + // Deliberately NOT beside `check:test-typecheck` in GATED above, and the + // difference is the whole design of #5475: that gate compares a checked-in + // artifact (test-typecheck-debt.json) against a fresh tsc run, so it has a + // generator and a directional ratchet. This one has NEITHER — `scripts/**` + // entered its program with zero ledger entries and is meant to stay there, so + // there is no file to regenerate and no `--fix` that could make it green. A + // failure here is always a code change. + { + check: 'check:scripts-typecheck', + why: 'type-checks packages/spec/scripts/** (the generators and gate scripts themselves) under tsconfig.scripts.json — no artifact, and no debt ledger by design (#5475)', + }, ]; /** diff --git a/packages/spec/scripts/generate-sbom.ts b/packages/spec/scripts/generate-sbom.ts index 9556bda717..fe1384abf0 100644 --- a/packages/spec/scripts/generate-sbom.ts +++ b/packages/spec/scripts/generate-sbom.ts @@ -11,7 +11,6 @@ import fs from 'fs'; import path from 'path'; const ROOT = path.resolve(__dirname, '..'); -const PACKAGES_DIR = path.resolve(ROOT, '..'); // packages/ interface SBOMComponent { type: string; diff --git a/packages/spec/scripts/liveness/check-liveness.mts b/packages/spec/scripts/liveness/check-liveness.mts index 079f4396ba..5284e47ec8 100644 --- a/packages/spec/scripts/liveness/check-liveness.mts +++ b/packages/spec/scripts/liveness/check-liveness.mts @@ -670,10 +670,16 @@ if (asJson) { ); } // ── re-verification clock ── - const v = report.verification!; + // Annotated at the boundary: `report` is deliberately `any` (see its + // declaration), so without this every `v.*` below is `any` too — which is how + // the `stale` worklist ended up iterated with an implicitly-any element while + // its neighbours carried hand-written `: string` annotations (#5475). Naming + // the producer's own type once types all four reads, and a shape change in + // verification.mts now lands here instead of passing through. + const v: VerificationReport = report.verification!; if (v.errors.length) { console.log(`\n✗ ${v.errors.length} malformed \`verifiedAt\` value(s) — a bad date silently disables the staleness check:`); - v.errors.forEach((s: string) => console.log(` ${s}`)); + v.errors.forEach((s) => console.log(` ${s}`)); } const dated = v.fresh + v.stale.length; console.log( @@ -687,7 +693,7 @@ if (asJson) { } if (v.unverified.length) { console.log(`\n never dated (${v.unverified.length}) — predate the field; date them as you re-verify:`); - v.unverified.forEach((k: string) => console.log(` ${k}`)); + v.unverified.forEach((k) => console.log(` ${k}`)); } } else if (v.stale.length || v.unverified.length) { console.log(' run with --stale-verification[=days] for the worklist.'); diff --git a/packages/spec/tsconfig.scripts.json b/packages/spec/tsconfig.scripts.json new file mode 100644 index 0000000000..22bb5ce780 --- /dev/null +++ b/packages/spec/tsconfig.scripts.json @@ -0,0 +1,65 @@ +// The SCRIPTS-layer type-check program (#5475). Third and last of the shapes +// this package's tsc coverage could be missing: #4311 was "the package has no +// typecheck task at all", #5286 was "the test layer is EXCLUDED from the one it +// has", and this is "the directory was never INCLUDED by anything". Not the +// same defect twice — an exclusion is at least visible in a config, while +// `scripts/` simply appeared in no `include` in the package, so +// `check:type-check-coverage` could not even count it (that gate tallies test +// files under the PRIMARY config's include roots, and `src` is where those stop). +// +// What is in here, and why it is not dead weight: +// - 29 `scripts/**/*.test.ts` files that `vitest.config.ts` genuinely runs on +// every `pnpm test`; +// - ~48 more files that ARE the gates: `build-schemas.ts` (`gen:schema`, the +// producer of every JSON Schema this package publishes, and of the #4650 +// authorable-surface deletion gate's verdict), `build-openapi.ts`, +// `liveness/check-liveness.mts`, `check-strictness-ledger.mts`, and the +// twenty-odd others `package.json` wires to a `check:*` script. +// These decide the package's generated artifacts and several gates' red/green, +// and until this file existed no type checker had read one line of them. tsx, +// which runs them, only transpiles. +// +// Why a SIBLING of `tsconfig.test.json` rather than a wider `include` on it: +// - `allowImportingTsExtensions` is needed here and ONLY here. The `.mts` +// modules under `scripts/liveness/` import each other by TS extension +// (`./evidence.mts`), which tsx resolves and tsc rejects without the flag. +// Turning it on for the test program would additionally permit +// `import './x.ts'` inside `src/**/*.test.ts`, where the BUILD config — +// which emits — still rejects it. A flag that buys nothing for `src` and +// opens a new divergence there belongs on the program that needs it. +// - `test-typecheck-debt.json` is ONE exact ledger per package, keyed by file. +// Its 79 entries are `src/**/*.test.ts` fixtures, and the ledger is +// shrink-only in both directions. `scripts/**` enters with ZERO entries — +// every error the merge surfaced is FIXED in the change that added this +// file, not recorded — so a plain `tsc --noEmit` is the stricter gate: the +// ledger permits growth by adding a line, this permits none. +// +// STRICTNESS IS UNTOUCHED, the same commitment `tsconfig.test.json` makes and +// for the same reason: `strict`, `noUnusedLocals`, `noUnusedParameters`, +// `noImplicitReturns` and the rest are inherited from the root config. Only +// module semantics and the program's root move. If a script does not compile, +// that is the finding — it is how this change learned that ~100 lines of +// `build-schemas.ts` were being checked as `never` (see that file's +// `gitResolvedAnchor` note). +// +// `module`/`moduleResolution` match `tsconfig.test.json`'s reasoning applied to +// the other runner: these files execute under tsx, which resolves extensionless +// relative imports and TS extensions alike. The build config's NodeNext would +// report that spelling as an error about the CHECK rather than about the code. +// +// `rootDir` widens to the package root because `scripts/` sits outside `src/`. +// It is a program-shape statement only — `noEmit` is on, nothing is written. +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "noEmit": true, + "rootDir": ".", + "module": "esnext", + "moduleResolution": "bundler", + "allowImportingTsExtensions": true, + "lib": ["ES2022", "DOM", "DOM.Iterable"], + "types": ["node"] + }, + "include": ["scripts/**/*"], + "exclude": ["node_modules", "dist"] +} diff --git a/packages/spec/tsconfig.test.json b/packages/spec/tsconfig.test.json index f15cc97ef1..42b8a376fc 100644 --- a/packages/spec/tsconfig.test.json +++ b/packages/spec/tsconfig.test.json @@ -16,11 +16,13 @@ // 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. +// `scripts/` — which vitest also runs, and which holds every generator and gate +// script — is the sibling `tsconfig.scripts.json` (#5475). It stayed a separate +// program rather than a wider `include` here for two measured reasons: it needs +// `allowImportingTsExtensions`, which would buy `src` nothing and would let a +// `src/**/*.test.ts` import `./x.ts` in a spelling the emitting build config +// still rejects; and it carries no ledger entries at all, so plain `tsc` is a +// stricter gate for it than this file's shrink-only debt list. { "extends": "./tsconfig.json", "compilerOptions": { diff --git a/scripts/check-regen-pending.d.mts b/scripts/check-regen-pending.d.mts new file mode 100644 index 0000000000..2cc0c279af --- /dev/null +++ b/scripts/check-regen-pending.d.mts @@ -0,0 +1,32 @@ +// Types for the two freshness predicates `check-regen-pending.mjs` exports to +// other gates (#5475). +// +// The module itself stays `.mjs`: `pre-commit` and `check:merge-driver` invoke +// it with bare `node`, and every root script here is authored that way. What +// changed is that three files under `packages/spec/scripts/` import from it — +// `build-docs.ts`, `check-generated.ts` and `schema-tree-freshness.test.ts` — +// and since #5475 those are inside a tsc program (`tsconfig.scripts.json`), +// where an untyped `.mjs` import is TS7016: the predicate silently becomes +// `any`, and `if (distIsStale)` — the missing call, the exact mistake this +// guard exists to prevent — would type-check clean. +// +// Declared rather than inferred (no `allowJs`) because the module sits at the +// repo root, outside the consuming program's `rootDir`. The surface is two +// functions with one optional argument; keep this file in step with them by +// hand, and keep it small enough that doing so stays trivial. + +/** + * Is `packages/spec/dist` older than the sources it claims to describe? + * Missing counts as stale. + * + * @param specDir Absolute path to the spec package; defaults to this repo's. + */ +export function distIsStale(specDir?: string): boolean; + +/** + * Is `packages/spec/json-schema` older than the sources it was generated from? + * Missing counts as stale. + * + * @param specDir Absolute path to the spec package; defaults to this repo's. + */ +export function schemaTreeIsStale(specDir?: string): boolean;