Skip to content

Commit c24b313

Browse files
claude[bot]claude
andauthored
fix(spec): packages/spec/scripts/** 进入类型检查程序 —— 新增 tsconfig.scripts.json,修掉全部 43 条错误 (#6398)
`packages/spec/tsconfig.json` 的 include 是 `["src/**/*"]`,#5286 的 `tsconfig.test.json` 同样止于 `src`。于是 `scripts/**` 不在本包任何 tsconfig 的 program 里 —— 不是「被 exclude 排除」,而是压根没被 include 选进来。29 个 `scripts/**/*.test.ts` 由 vitest 真的在跑,另有约 48 个文件就是 `gen:schema` / `gen:openapi` / `check:liveness` / `check:strictness-ledger` 等门禁本体;tsx 只转译。 新增 `tsconfig.scripts.json`(strictness 全部继承,未放松任何一项),由 `check:scripts-typecheck` 指名,并接入 `typecheck` 脚本链。合并后实测 43 条错误, 本 PR 全部修完,`scripts/**` 以零台账条目进入 —— 因此 plain tsc 比 shrink-only 台账更严:台账允许加一行来增长,这里一条也不允许。 其中 `build-schemas.ts` 的 13 条 `Property 'keys'/'rev' does not exist on type 'never'` 是本单最有价值的产出:`gitResolvedAnchor` 是模块级 `let ... = null`, 唯一赋值在 `resolveSurfaceBase()` 函数体内,而 TypeScript 的控制流分析不跟踪函数体 内的赋值,于是每个顶层读取处它仍被收窄为 `null` —— `if (gitResolvedAnchor)` 的整个 块体类型为 `never`,即 #5235/#5358/#5370/#5847 那约 100 行 in-tree anchor 写入逻辑 从未被类型检查过。改为经返回值 `SurfaceBaseResolution.gitAnchor` 传出,赋值回到调用方 自己的控制流里。运行时行为不变。 Fixes #5475 Claude-Session: https://claude.ai/code/session_014wsZeReNTqiceBfLb5Pyf5 Co-authored-by: Claude <noreply@anthropic.com>
1 parent a841151 commit c24b313

11 files changed

Lines changed: 206 additions & 43 deletions

packages/spec/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,8 @@
220220
"check:skill-examples": "tsx scripts/check-skill-examples.ts --self-test && tsx scripts/check-skill-examples.ts",
221221
"check:test-typecheck": "tsx ../../scripts/check-test-typecheck.mts --self-test && tsx ../../scripts/check-test-typecheck.mts --package packages/spec --project tsconfig.test.json",
222222
"gen:test-typecheck-debt": "tsx ../../scripts/check-test-typecheck.mts --update --package packages/spec --project tsconfig.test.json",
223-
"typecheck": "tsc --noEmit && pnpm check:test-typecheck"
223+
"check:scripts-typecheck": "tsc --noEmit -p tsconfig.scripts.json",
224+
"typecheck": "tsc --noEmit && pnpm check:scripts-typecheck && pnpm check:test-typecheck"
224225
},
225226
"keywords": [
226227
"objectstack",

packages/spec/scripts/authorable-defaults.test.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,10 +87,15 @@ describe('collectAuthorableDefaults — what the fingerprint reads', () => {
8787
// loudly, so it is a different and self-announcing class. If this ever
8888
// fails, someone has widened the ratchet into direction A — which was
8989
// considered and declined, not overlooked.
90-
const loose = [
90+
// Typed as the collector's own parameter, not `as const`: a `readonly`
91+
// tuple is not an `Iterable<[string, unknown]>`, so these two fixtures did
92+
// not type-check at all — invisible until #5475 put `scripts/` in a tsc
93+
// program. Naming the producer's type also keeps the fixture honest if that
94+
// signature ever changes.
95+
const loose: Array<[string, unknown]> = [
9196
['system/Job', { properties: { maxRetries: { type: 'integer', minimum: 0, default: 0 } } }],
92-
] as const;
93-
const tightened = [
97+
];
98+
const tightened: Array<[string, unknown]> = [
9499
[
95100
'system/Job',
96101
{
@@ -105,7 +110,7 @@ describe('collectAuthorableDefaults — what the fingerprint reads', () => {
105110
},
106111
},
107112
],
108-
] as const;
113+
];
109114
expect([...collectAuthorableDefaults(tightened)]).toEqual([...collectAuthorableDefaults(loose)]);
110115
});
111116

packages/spec/scripts/build-docs.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,11 @@ function sourcePathToDocsRoute(target: string): string | null {
300300
// because a helper happened to sit at the top of the file, and `check:docs`
301301
// could not see it (the artifact reproduced the wrong block faithfully).
302302

303-
function generateMarkdown(schemaName: string, schema: any, category: string, zodFile: string) {
303+
// `_zodFile` is passed by the caller and deliberately unread here: the file slug
304+
// is a page-level fact, and every use of it (title, source link, card) lives in
305+
// `generateZodFileMarkdown` around this call. Underscored rather than dropped so
306+
// this touches one line of a renderer PR #6377 is editing (#5475).
307+
function generateMarkdown(schemaName: string, schema: any, category: string, _zodFile: string) {
304308
const defs = schema.definitions || schema.$defs || {};
305309
let mainDef = defs[schemaName];
306310

@@ -721,8 +725,6 @@ Object.keys(CATEGORIES).forEach(category => {
721725
managedCount++;
722726
});
723727

724-
const generatedFiles: string[] = [];
725-
726728
// 2. Generate Files
727729
// Clear DOCS_ROOT first to remove old flattened files
728730
if (fs.existsSync(DOCS_ROOT)) {

packages/spec/scripts/build-openapi.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,12 @@ import { z } from 'zod';
66

77
// Dynamic imports from spec source
88
import * as API from '../src/api';
9-
import * as Data from '../src/data';
9+
// `import * as Data from '../src/data'` used to sit here, bound and never read.
10+
// It was already a no-op at runtime — TS import elision drops an unused
11+
// namespace import before tsx ever evaluates it — so this removes a name, not a
12+
// side effect; `json-schema/openapi.json` is byte-identical across the change
13+
// (#5475). Restoring the module evaluation, had it been load-bearing, would
14+
// have meant a bare `import '../src/data';`, which is a different statement.
1015
import { assertRefsResolve, assertNoDegradedSchemas } from './lib/openapi-self-consistency';
1116
// The name this generator writes is DECLARED next to `build-schemas.ts`'s clean
1217
// step, which shares this directory and must not sweep it away (#5371). Imported

packages/spec/scripts/build-schemas.ts

Lines changed: 60 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,7 @@ import {
2727
clearOwnedOutputs,
2828
} from './lib/json-schema-out-dir';
2929
import {
30-
AUTHORABLE_SURFACE_DESCRIPTION,
3130
AUTHORABLE_SURFACE_DIR_NAME,
32-
SCHEMA_MANIFEST_DESCRIPTION,
3331
SCHEMA_MANIFEST_DIR_NAME,
3432
aggregateCategoryShards,
3533
authorableSurfaceShardTexts,
@@ -38,6 +36,7 @@ import {
3836
serializeShard,
3937
writeShards,
4038
type GitRun,
39+
type ShardArrayField,
4140
} from './lib/sharded-artifacts';
4241
// The #4666 default-value ratchet: what an author gets when they OMIT a key.
4342
// Its own module because the fingerprint's normalisation rules — and the
@@ -483,15 +482,11 @@ if (defKeyCollisions.length > 0) {
483482
// run means a code change unpublished a schema — fail loudly instead of
484483
// letting gen:docs quietly delete its reference docs (#2978). Deliberate
485484
// removals must delete the key from the manifest in the same PR.
486-
/**
487-
* The manifest's description — the procedure a reader who opens a shard to
488-
* delete a line follows. Until #4725 it ended "remove a key ONLY for a
489-
* deliberate retirement", which was the entire requirement and was checked by
490-
* nothing; it now names the gate and the table that answer for a removal. It
491-
* lives in scripts/lib/sharded-artifacts.ts with the writer that stamps it into
492-
* every shard (#5837).
493-
*/
494-
const MANIFEST_DESCRIPTION = SCHEMA_MANIFEST_DESCRIPTION;
485+
// The manifest's and the authorable surface's shard descriptions used to be
486+
// re-exported through here. #5837 moved both to scripts/lib/sharded-artifacts.ts,
487+
// beside the writer that stamps them into every shard, and nothing in this file
488+
// has read them since — the import and the `MANIFEST_DESCRIPTION` alias were
489+
// residue no checker could see (#5475).
495490

496491
/**
497492
* Every def key recorded across `json-schema.manifest/`, or null when the whole
@@ -1094,7 +1089,13 @@ function readSurfaceKeysAtRev(
10941089
git: GitRun,
10951090
rev: string,
10961091
dirName: string,
1097-
field: 'keys' | 'schemas',
1092+
// `ShardArrayField`, not a re-spelled copy of it. This parameter used to read
1093+
// `'keys' | 'schemas'` — a hand-written narrowing of the exported union that
1094+
// `readShardedKeysAtRev` below actually takes. When #4666 added `'defaults'`
1095+
// to `ShardArrayField` and a call site passing it, the copy here was left
1096+
// behind and no type checker existed to say so (#5475). Harmless at runtime,
1097+
// since the value is only forwarded, but it is the drift this program is for.
1098+
field: ShardArrayField,
10981099
context: string,
10991100
): { entries: string[] } | null {
11001101
const read = readShardedKeysAtRev(git, rev, dirName, field);
@@ -1453,12 +1454,33 @@ function assertAnchorMovesForward(git: GitRun, committedRev: string, resolvedRev
14531454
}
14541455

14551456
/**
1456-
* Set when THIS run resolved the baseline from git. It is the ONLY input
1457-
* `--update-base` may write the in-tree anchor from: an offline build must never
1458-
* be able to advance the anchor to its own state (#5235). The second half of that
1459-
* discipline is #5358 — no build writes it at all, only the explicit mode.
1457+
* What `resolveSurfaceBase()` resolved: the baseline itself, plus — only when
1458+
* the GIT path produced it — the anchor that path is allowed to write.
1459+
*
1460+
* `gitAnchor` is a returned field rather than the module-level assignment it
1461+
* used to be, and that is a type-checking fix, not a style one (#5475). The old
1462+
* shape declared `let gitResolvedAnchor: {...} | null = null` here and assigned
1463+
* it from INSIDE this function. TypeScript's control-flow analysis does not
1464+
* follow an assignment made in a function body, so at every top-level read below
1465+
* the variable was still narrowed to `null` — which made `if (gitResolvedAnchor)`
1466+
* a block whose body is typed `never`, i.e. the entire in-tree anchor writer
1467+
* (#5235/#5358/#5370/#5847, ~100 lines) was invisible to tsc while reading as
1468+
* ordinary checked code. Returning the value puts the assignment in the caller's
1469+
* own flow, where CFA can see it. Runtime behaviour is unchanged: the git path
1470+
* sets it, the in-tree path leaves it null, exactly as before.
14601471
*/
1461-
let gitResolvedAnchor: { rev: string; keys: string[] } | null = null;
1472+
type SurfaceBaseResolution = {
1473+
rev: string;
1474+
doc: AuthorableSurface;
1475+
/**
1476+
* Set when THIS run resolved the baseline from git. It is the ONLY input
1477+
* `--update-base` may write the in-tree anchor from: an offline build must
1478+
* never be able to advance the anchor to its own state (#5235). The second
1479+
* half of that discipline is #5358 — no build writes it at all, only the
1480+
* explicit mode.
1481+
*/
1482+
gitAnchor: { rev: string; keys: string[] } | null;
1483+
};
14621484

14631485
/**
14641486
* The committed authorable surface this PR started from: its content at
@@ -1483,7 +1505,7 @@ let gitResolvedAnchor: { rev: string; keys: string[] } | null = null;
14831505
* What is NOT offered is an env-var skip: that is precisely the bypass #4650
14841506
* closes. With no anchor of either kind this still exits 1.
14851507
*/
1486-
function resolveSurfaceBase(): { rev: string; doc: AuthorableSurface } | null {
1508+
function resolveSurfaceBase(): SurfaceBaseResolution | null {
14871509
const git = gitInPackage;
14881510
const committed = readCommittedSurfaceBase();
14891511

@@ -1520,10 +1542,10 @@ function resolveSurfaceBase(): { rev: string; doc: AuthorableSurface } | null {
15201542
return null;
15211543
}
15221544
const doc: AuthorableSurface = { keys: baseline.entries };
1523-
gitResolvedAnchor = { rev, keys: doc.keys };
1545+
const gitAnchor = { rev, keys: doc.keys };
15241546
// The environment that CAN police the in-tree anchor is the one that must.
1525-
if (committed) verifyCommittedSurfaceBase(git, tip, gitResolvedAnchor, committed.doc);
1526-
return { rev, doc };
1547+
if (committed) verifyCommittedSurfaceBase(git, tip, gitAnchor, committed.doc);
1548+
return { rev, doc, gitAnchor };
15271549
}
15281550

15291551
if (committed) {
@@ -1535,6 +1557,9 @@ function resolveSurfaceBase(): { rev: string; doc: AuthorableSurface } | null {
15351557
return {
15361558
rev: committed.doc.baseRev,
15371559
doc: { keys: committed.doc.keys },
1560+
// Offline: this run did not resolve an anchor from git, so it has nothing
1561+
// it is entitled to write one from (#5235).
1562+
gitAnchor: null,
15381563
};
15391564
}
15401565

@@ -1722,11 +1747,20 @@ function checkManifestRemovals(git: GitRun, baseRev: string | null): void {
17221747
* is one resolution, shared — a second `resolveSurfaceBase()` call would ask git
17231748
* the same question twice and could answer it differently.
17241749
*/
1725-
let resolvedSurfaceBase: { rev: string; doc: AuthorableSurface } | null = null;
1750+
let resolvedSurfaceBase: SurfaceBaseResolution | null = null;
1751+
1752+
/**
1753+
* The git-resolved anchor of this run, hoisted out of the block below because
1754+
* the in-tree anchor writer further down is a separate top-level block.
1755+
* Assigned HERE, in the module's own control flow, which is what keeps it typed
1756+
* as the union it is declared as — see `SurfaceBaseResolution.gitAnchor`.
1757+
*/
1758+
let gitResolvedAnchor: { rev: string; keys: string[] } | null = null;
17261759

17271760
{
17281761
const base = resolveSurfaceBase();
17291762
resolvedSurfaceBase = base;
1763+
gitResolvedAnchor = base?.gitAnchor ?? null;
17301764
// Whole defs first: check (c) below waives every baseline line under a def this
17311765
// build stopped emitting, on the grounds that this gate adjudicates it. Running
17321766
// it first is what makes that deferral true rather than circular.
@@ -1748,9 +1782,10 @@ let resolvedSurfaceBase: { rev: string; doc: AuthorableSurface } | null = null;
17481782
const violations: string[] = [];
17491783
const goneDefs = new Map<string, number>(); // def no longer emitted -> deleted key count
17501784
for (const key of deletedKeys) {
1751-
const sep = key.indexOf(':');
1752-
const defKey = key.slice(0, sep);
1753-
const prop = key.slice(sep + 1);
1785+
// Only the def half is read now. The leaf half fed the leaf-NAME match
1786+
// #5898 removed from route 3 (see the RETIRED_KEYS_BY_MAJOR message
1787+
// below); slicing it out survived the rewrite as a dead local (#5475).
1788+
const defKey = key.slice(0, key.indexOf(':'));
17541789
if (!generatedSchemas.has(defKey)) {
17551790
goneDefs.set(defKey, (goneDefs.get(defKey) ?? 0) + 1);
17561791
continue;

packages/spec/scripts/check-generated.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,17 @@ const NO_GENERATOR: ReadonlyArray<{ check: string; why: string }> = [
173173
check: 'check:dual-source-exports',
174174
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`)',
175175
},
176+
// Deliberately NOT beside `check:test-typecheck` in GATED above, and the
177+
// difference is the whole design of #5475: that gate compares a checked-in
178+
// artifact (test-typecheck-debt.json) against a fresh tsc run, so it has a
179+
// generator and a directional ratchet. This one has NEITHER — `scripts/**`
180+
// entered its program with zero ledger entries and is meant to stay there, so
181+
// there is no file to regenerate and no `--fix` that could make it green. A
182+
// failure here is always a code change.
183+
{
184+
check: 'check:scripts-typecheck',
185+
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)',
186+
},
176187
];
177188

178189
/**

packages/spec/scripts/generate-sbom.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import fs from 'fs';
1111
import path from 'path';
1212

1313
const ROOT = path.resolve(__dirname, '..');
14-
const PACKAGES_DIR = path.resolve(ROOT, '..'); // packages/
1514

1615
interface SBOMComponent {
1716
type: string;

packages/spec/scripts/liveness/check-liveness.mts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -670,10 +670,16 @@ if (asJson) {
670670
);
671671
}
672672
// ── re-verification clock ──
673-
const v = report.verification!;
673+
// Annotated at the boundary: `report` is deliberately `any` (see its
674+
// declaration), so without this every `v.*` below is `any` too — which is how
675+
// the `stale` worklist ended up iterated with an implicitly-any element while
676+
// its neighbours carried hand-written `: string` annotations (#5475). Naming
677+
// the producer's own type once types all four reads, and a shape change in
678+
// verification.mts now lands here instead of passing through.
679+
const v: VerificationReport = report.verification!;
674680
if (v.errors.length) {
675681
console.log(`\n✗ ${v.errors.length} malformed \`verifiedAt\` value(s) — a bad date silently disables the staleness check:`);
676-
v.errors.forEach((s: string) => console.log(` ${s}`));
682+
v.errors.forEach((s) => console.log(` ${s}`));
677683
}
678684
const dated = v.fresh + v.stale.length;
679685
console.log(
@@ -687,7 +693,7 @@ if (asJson) {
687693
}
688694
if (v.unverified.length) {
689695
console.log(`\n never dated (${v.unverified.length}) — predate the field; date them as you re-verify:`);
690-
v.unverified.forEach((k: string) => console.log(` ${k}`));
696+
v.unverified.forEach((k) => console.log(` ${k}`));
691697
}
692698
} else if (v.stale.length || v.unverified.length) {
693699
console.log(' run with --stale-verification[=days] for the worklist.');
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
// The SCRIPTS-layer type-check program (#5475). Third and last of the shapes
2+
// this package's tsc coverage could be missing: #4311 was "the package has no
3+
// typecheck task at all", #5286 was "the test layer is EXCLUDED from the one it
4+
// has", and this is "the directory was never INCLUDED by anything". Not the
5+
// same defect twice — an exclusion is at least visible in a config, while
6+
// `scripts/` simply appeared in no `include` in the package, so
7+
// `check:type-check-coverage` could not even count it (that gate tallies test
8+
// files under the PRIMARY config's include roots, and `src` is where those stop).
9+
//
10+
// What is in here, and why it is not dead weight:
11+
// - 29 `scripts/**/*.test.ts` files that `vitest.config.ts` genuinely runs on
12+
// every `pnpm test`;
13+
// - ~48 more files that ARE the gates: `build-schemas.ts` (`gen:schema`, the
14+
// producer of every JSON Schema this package publishes, and of the #4650
15+
// authorable-surface deletion gate's verdict), `build-openapi.ts`,
16+
// `liveness/check-liveness.mts`, `check-strictness-ledger.mts`, and the
17+
// twenty-odd others `package.json` wires to a `check:*` script.
18+
// These decide the package's generated artifacts and several gates' red/green,
19+
// and until this file existed no type checker had read one line of them. tsx,
20+
// which runs them, only transpiles.
21+
//
22+
// Why a SIBLING of `tsconfig.test.json` rather than a wider `include` on it:
23+
// - `allowImportingTsExtensions` is needed here and ONLY here. The `.mts`
24+
// modules under `scripts/liveness/` import each other by TS extension
25+
// (`./evidence.mts`), which tsx resolves and tsc rejects without the flag.
26+
// Turning it on for the test program would additionally permit
27+
// `import './x.ts'` inside `src/**/*.test.ts`, where the BUILD config —
28+
// which emits — still rejects it. A flag that buys nothing for `src` and
29+
// opens a new divergence there belongs on the program that needs it.
30+
// - `test-typecheck-debt.json` is ONE exact ledger per package, keyed by file.
31+
// Its 79 entries are `src/**/*.test.ts` fixtures, and the ledger is
32+
// shrink-only in both directions. `scripts/**` enters with ZERO entries —
33+
// every error the merge surfaced is FIXED in the change that added this
34+
// file, not recorded — so a plain `tsc --noEmit` is the stricter gate: the
35+
// ledger permits growth by adding a line, this permits none.
36+
//
37+
// STRICTNESS IS UNTOUCHED, the same commitment `tsconfig.test.json` makes and
38+
// for the same reason: `strict`, `noUnusedLocals`, `noUnusedParameters`,
39+
// `noImplicitReturns` and the rest are inherited from the root config. Only
40+
// module semantics and the program's root move. If a script does not compile,
41+
// that is the finding — it is how this change learned that ~100 lines of
42+
// `build-schemas.ts` were being checked as `never` (see that file's
43+
// `gitResolvedAnchor` note).
44+
//
45+
// `module`/`moduleResolution` match `tsconfig.test.json`'s reasoning applied to
46+
// the other runner: these files execute under tsx, which resolves extensionless
47+
// relative imports and TS extensions alike. The build config's NodeNext would
48+
// report that spelling as an error about the CHECK rather than about the code.
49+
//
50+
// `rootDir` widens to the package root because `scripts/` sits outside `src/`.
51+
// It is a program-shape statement only — `noEmit` is on, nothing is written.
52+
{
53+
"extends": "./tsconfig.json",
54+
"compilerOptions": {
55+
"noEmit": true,
56+
"rootDir": ".",
57+
"module": "esnext",
58+
"moduleResolution": "bundler",
59+
"allowImportingTsExtensions": true,
60+
"lib": ["ES2022", "DOM", "DOM.Iterable"],
61+
"types": ["node"]
62+
},
63+
"include": ["scripts/**/*"],
64+
"exclude": ["node_modules", "dist"]
65+
}

packages/spec/tsconfig.test.json

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,13 @@
1616
// may loosen a type rule; if a test does not compile, that is the finding.
1717
//
1818
// `include` deliberately stops at `src`, matching the build config's root.
19-
// `scripts/` holds nine more test files vitest runs and is in no tsconfig at
20-
// all — a second, differently-shaped hole (measured at 16 files / 33 errors,
21-
// mostly config-tier TS5097/TS2593 plus a real TS2339 pile in build-schemas.ts)
22-
// that wants its own change rather than a rider on this one. None of those
23-
// files carries a `@ts-expect-error`, so no pin is hiding there.
19+
// `scripts/` — which vitest also runs, and which holds every generator and gate
20+
// script — is the sibling `tsconfig.scripts.json` (#5475). It stayed a separate
21+
// program rather than a wider `include` here for two measured reasons: it needs
22+
// `allowImportingTsExtensions`, which would buy `src` nothing and would let a
23+
// `src/**/*.test.ts` import `./x.ts` in a spelling the emitting build config
24+
// still rejects; and it carries no ledger entries at all, so plain `tsc` is a
25+
// stricter gate for it than this file's shrink-only debt list.
2426
{
2527
"extends": "./tsconfig.json",
2628
"compilerOptions": {

0 commit comments

Comments
 (0)