Skip to content

Commit 7617956

Browse files
os-zhuangclaude
andauthored
fix(spec): 让 spec 测试层真的进 tsc,@ts-expect-error 退役 pin 不再是幽灵检查 (#5478)
* fix(spec): compile the test layer so `@ts-expect-error` retirement pins stop being phantom checks `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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018fxLGQdatPbBUvCgiVxg6D * docs(spec): explain the tsconfig.test.json boundary and point the phantom-pin baseline at its issue - tsconfig.test.json gains the reasoning it was missing: what differs from the build config (module semantics only) and what deliberately does not (strictness, inherited untouched). `include` stops at `src`, and the comment says why: `packages/spec/scripts/**` is in no tsconfig at all — a second, differently-shaped hole, measured at 16 files / 33 errors and filed as #5475 rather than ridden along here. No `@ts-expect-error` hides there. - PHANTOM_PIN_DEBT's metadata-core entry now names #5476, the issue that closes it. Its cause differs from spec's: no exclusion names the file, it simply sits outside `include`, which is why TESTS_COVERED could never see it either. - Drop an unused constant from check-test-typecheck.mts (found by compiling the scripts layer during the measurement above). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018fxLGQdatPbBUvCgiVxg6D * chore(spec): classify the test-layer typecheck gate in the check:generated ledger `check:generated --reconcile-only` — the required meta-gate in lint.yml's TypeScript Type Check job — went red on this branch. `tsc --noEmit` passed; the step after it did not. package.json declared `check:test-typecheck` and `gen:test-typecheck-debt`, and no bucket in check-generated.ts named either: `check:test-typecheck` exists in package.json but is in neither GATED nor NO_GENERATOR. `gen:test-typecheck-debt` exists in package.json but no GATED entry names it [...] They are a GATED pair. The gate compares a checked-in artifact (test-typecheck-debt.json) against what `tsc -p tsconfig.test.json` measures right now, and `gen:test-typecheck-debt` is that artifact's writer — so NO_GENERATOR ("no artifact to regenerate") and UNGATED_GENERATORS ("nothing verifies this output") would each have been false, in opposite directions. What did NOT fit the existing shape is `--fix`. Every other GATED artifact is a pure function of the source, so regenerating is always the right answer. This one records DEBT, and its four verdicts split two ways: "the debt shrank" and "the file graduated" mean re-record, while "the debt grew" and "an unledgered file has errors" mean fix the code. `--fix` regenerates without reading which one it got, and the gate's own failure text says the ledger "only ratchets down" — so a blind `--fix` would contradict the gate it is fixing and launder new debt in as a mechanical diff, the same hazard that keeps dual-source-exports.baseline.json out of GATED entirely (#4446). Hence a `ratchet` flag: the entry is GATED and reported like every other, and `--fix` refuses it, printing the gate's own prescription instead of guessing. This is not hypothetical — merging main into this branch brought three new spec test files in. They happened to compile clean (the ledger stayed byte-identical at 79 files / 691 errors), but had any of them carried errors, a reflexive `--fix` would have ledgered them silently. Same reasoning adds the artifact to NOT_DRIVER_MANAGED, beside docs-import-surface.baseline.json: a merge driver must not recompute a shrink-only ratchet either. Also pins the reconciliation itself in the spec suite. It had already been dormant or unsatisfied three times (#4177, #4232, and this branch), each costing a CI lap because `pnpm test` never read the ledger. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018fxLGQdatPbBUvCgiVxg6D --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 4d57387 commit 7617956

38 files changed

Lines changed: 1074 additions & 159 deletions

AGENTS.md

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,23 @@ that graduates deletes its ledger entry in the same PR.
3434
`typecheck` script advertises — a green gate over source nothing read, which is the
3535
#4311 defect itself. The ratchet's `TESTS_COVERED` invariant fails on any new exclusion;
3636
the packages that already had one carry a measured `TEST_DEBT` entry and graduate by
37-
dropping the exclusion.
37+
dropping the exclusion — or, when the build config must keep the exclusion (ci.yml gates
38+
that no test file reaches the published artifact), by adding a **sibling
39+
`tsconfig.test.json` and naming it in the `typecheck` script**, which is what
40+
`packages/spec` does since #5286. The sibling may carry its own *module* semantics to
41+
match how vitest executes the files (`module: esnext`, `moduleResolution: bundler`) —
42+
never its own *strictness*: `strict` and friends are inherited, untouched.
43+
44+
**A `@ts-expect-error` in a file no tsc program compiles is a phantom check** — the
45+
`PINS_CHECKED` invariant of the same ratchet, repo-wide. `@ts-expect-error` is the
46+
"tsc is the best sweeper" channel the spec-property-retirement playbook leans on: the
47+
directive is meant to go red the day a removed key comes back. Outside a program it
48+
evaluates never, and *deleting the directive leaves every gate just as green* — which is
49+
how spec's 17 retirement pins across 5 files were found (#5286). Before writing one,
50+
check the file is compiled. `packages/spec` additionally holds its test-layer residue in
51+
a per-file, exactly-measured, shrink-only ledger (`packages/spec/test-typecheck-debt.json`,
52+
`pnpm --filter @objectstack/spec gen:test-typecheck-debt`): a file not listed there may
53+
have no type errors at all.
3854

3955
One trap worth knowing before you read any of these counts: under `moduleResolution:
4056
NodeNext` a relative import missing its `.js` extension does not resolve, every symbol it

packages/spec/package.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,9 @@
217217
"check:react-blocks": "tsx scripts/build-react-blocks-contract.ts --check",
218218
"check:react-declaration-parity": "tsx scripts/check-react-blocks-declaration-parity.ts",
219219
"check:skill-examples": "tsx scripts/check-skill-examples.ts",
220-
"typecheck": "tsc --noEmit"
220+
"check:test-typecheck": "tsx scripts/check-test-typecheck.mts --self-test && tsx scripts/check-test-typecheck.mts --project tsconfig.test.json",
221+
"gen:test-typecheck-debt": "tsx scripts/check-test-typecheck.mts --update --project tsconfig.test.json",
222+
"typecheck": "tsc --noEmit && pnpm check:test-typecheck"
221223
},
222224
"keywords": [
223225
"objectstack",
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// Pins the `check:generated` LEDGER against package.json — the reconciliation
4+
// that decides whether every `check:`/`gen:` script in this package is actually
5+
// covered by a gate, or has quietly dropped out of coverage.
6+
//
7+
// WHY IT IS WORTH A TEST AND NOT JUST A CI STEP. The reconciliation has been
8+
// dormant or unsatisfied three times now, and each time the cost was a CI lap
9+
// rather than a local one:
10+
//
11+
// • #4177 and #4232 landed unclassified scripts while nothing in CI ran the
12+
// reconciliation at all, so `main` carried a wrapper that exited red before
13+
// running a single gate.
14+
// • #4291 fixed that by wiring `--reconcile-only` into lint.yml's unfiltered
15+
// required job — which is exactly where #5286's own first push then died,
16+
// because the two scripts it added (`check:test-typecheck`,
17+
// `gen:test-typecheck-debt`) were in neither ledger bucket. `tsc` passed;
18+
// the step after it did not.
19+
//
20+
// So the negative direction of this reconciliation is not hypothetical — it is
21+
// the reason this file exists, observed in production twice. What was missing
22+
// was a signal BEFORE the push: `pnpm --filter @objectstack/spec test` did not
23+
// read the ledger, so a script added in one file and unclassified in another was
24+
// invisible until a runner said so ten minutes later. This closes that gap.
25+
//
26+
// It runs the real script in place: `--reconcile-only` reads package.json and
27+
// the ledger arrays and exits — no gates, no build, no writes, sub-second — so
28+
// there is nothing to sandbox and no way for it to differ from what CI runs.
29+
30+
import { describe, it, expect } from 'vitest';
31+
import { spawnSync } from 'node:child_process';
32+
import { createRequire } from 'node:module';
33+
import fs from 'node:fs';
34+
import path from 'node:path';
35+
import { fileURLToPath } from 'node:url';
36+
37+
const HERE = path.dirname(fileURLToPath(import.meta.url));
38+
const SPEC = path.resolve(HERE, '..');
39+
40+
function runReconcile(): { status: number | null; output: string } {
41+
const require = createRequire(import.meta.url);
42+
const tsx = require.resolve('tsx/cli');
43+
const result = spawnSync(process.execPath, [tsx, path.join(HERE, 'check-generated.ts'), '--reconcile-only'], {
44+
cwd: SPEC,
45+
encoding: 'utf8',
46+
maxBuffer: 16 * 1024 * 1024,
47+
});
48+
if (result.error) throw result.error;
49+
return { status: result.status, output: `${result.stdout ?? ''}${result.stderr ?? ''}` };
50+
}
51+
52+
describe('check:generated --reconcile-only', () => {
53+
const scripts: Record<string, string> = JSON.parse(fs.readFileSync(path.join(SPEC, 'package.json'), 'utf8')).scripts;
54+
const checks = Object.keys(scripts).filter((n) => n.startsWith('check:'));
55+
const gens = Object.keys(scripts).filter((n) => n.startsWith('gen:'));
56+
57+
it('classifies every check:/gen: script this package declares', () => {
58+
const { status, output } = runReconcile();
59+
// The failure text is the useful part when this goes red: it names the
60+
// unclassified script and asks the classifying question.
61+
expect(output).not.toMatch(/is in neither GATED nor NO_GENERATOR/);
62+
expect(output).not.toMatch(/it is not in UNGATED_GENERATORS/);
63+
expect(status, output).toBe(0);
64+
});
65+
66+
it('reports the same script counts package.json actually declares', () => {
67+
// Derived from package.json rather than hardcoded, so adding a gate does not
68+
// churn this test — only FAILING to classify one does.
69+
const { output } = runReconcile();
70+
expect(output).toContain(`${checks.length} check: + ${gens.length} gen: scripts`);
71+
expect(output).toContain('all classified');
72+
});
73+
74+
it('covers the test-layer typecheck gate and its writer (#5286)', () => {
75+
// The specific pair that failed CI on this branch. Named here so a later
76+
// change that drops either script also has to come back through this file.
77+
expect(scripts['check:test-typecheck']).toBeDefined();
78+
expect(scripts['gen:test-typecheck-debt']).toBeDefined();
79+
expect(runReconcile().status).toBe(0);
80+
});
81+
});

packages/spec/scripts/check-generated.ts

Lines changed: 78 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@
1717
* never saw, so a real semantic change lands silently inside a mechanical diff.
1818
* What is worth automating is the *diagnosis* — which artifacts are stale, and
1919
* the exact command for each. `--fix` then regenerates **only** the ones this run
20-
* proved stale, and says so.
20+
* proved stale, and says so — minus the `ratchet` entries, whose gate has already
21+
* answered a question `--fix` would otherwise have to guess (see GATED below).
2122
*
2223
* Usage:
2324
* pnpm --filter @objectstack/spec check:generated # report every stale artifact
@@ -41,8 +42,18 @@ const pkgRoot = join(dirname(fileURLToPath(import.meta.url)), '..');
4142
* The gates that verify a checked-in artifact against its source, with the
4243
* generator that rewrites each. Order is the cheapest-first order a human would
4344
* want the answers in, not CI's.
45+
*
46+
* `ratchet` marks the entries whose artifact is a DIRECTIONAL debt ledger rather
47+
* than a descriptive snapshot of the source. For those, "stale" is ambiguous —
48+
* see the `--fix` loop, which refuses to guess.
4449
*/
45-
const GATED: ReadonlyArray<{ check: string; gen: string; artifact: string; readsDist?: true }> = [
50+
const GATED: ReadonlyArray<{
51+
check: string;
52+
gen: string;
53+
artifact: string;
54+
readsDist?: true;
55+
ratchet?: true;
56+
}> = [
4657
{ check: 'check:spec-changes', gen: 'gen:spec-changes', artifact: 'spec-changes.json' },
4758
{ check: 'check:upgrade-guide', gen: 'gen:upgrade-guide', artifact: 'docs/protocol-upgrade-guide.md' },
4859
{ check: 'check:skill-docs', gen: 'gen:skill-docs', artifact: 'skill docs (from SKILL.md frontmatter)' },
@@ -69,6 +80,31 @@ const GATED: ReadonlyArray<{ check: string; gen: string; artifact: string; reads
6980
gen: 'gen:strictness-ledger',
7081
artifact: 'docs/audits/2026-07-unknown-key-strictness-ledger.counts.md',
7182
},
83+
// GATED by the definition above — it compares a checked-in artifact
84+
// (test-typecheck-debt.json) against what `tsc -p tsconfig.test.json` measures
85+
// right now, and `gen:test-typecheck-debt` is that artifact's writer. It is NOT
86+
// a source audit: there is a real file to regenerate, so NO_GENERATOR would be
87+
// a false classification, and UNGATED_GENERATORS ("nothing verifies this
88+
// output") would be false in the other direction.
89+
//
90+
// What it is, that nothing above it is, is a DIRECTIONAL ratchet — hence
91+
// `ratchet`. The other artifacts here are pure functions of the source, so
92+
// regenerating is always the right answer. This one records DEBT, and its four
93+
// verdicts split two ways: "the debt shrank" and "the file graduated" are
94+
// re-record, while "the debt grew" and "an unledgered file has errors" are fix
95+
// the code (#5286). `--fix` regenerates without reading which one it got, so
96+
// for this entry it refuses instead — the merge that brought three new spec
97+
// test files into this very branch is the live shape of the risk: had any of
98+
// them carried errors, a reflexive `--fix` would have ledgered them silently.
99+
//
100+
// Cost: this is the only gate here that runs a full tsc program (~30s over
101+
// src/**/*.test.ts), so it goes last in the cheapest-first order above.
102+
{
103+
check: 'check:test-typecheck',
104+
gen: 'gen:test-typecheck-debt',
105+
artifact: 'test-typecheck-debt.json',
106+
ratchet: true,
107+
},
72108
];
73109

74110
/**
@@ -237,18 +273,54 @@ if (!stale.length) {
237273
}
238274

239275
console.log(`\n✗ ${stale.length} of ${GATED.length} artifact(s) stale:\n`);
240-
for (const s of stale) console.log(` ${s.artifact}\n pnpm --filter @objectstack/spec ${s.gen}`);
276+
for (const s of stale) {
277+
console.log(` ${s.artifact}\n pnpm --filter @objectstack/spec ${s.gen}` +
278+
(s.ratchet ? ` ← only if ${s.check} asked you to RE-RECORD; --fix will not run this one` : ''));
279+
}
280+
281+
const autoFixable = stale.filter((s) => !s.ratchet);
241282

242283
if (!fix) {
243284
console.log(`\nRegenerate exactly these:\n ` +
244-
stale.map((s) => `pnpm --filter @objectstack/spec ${s.gen}`).join(' && ') +
245-
`\n\nOr re-run with --fix to do it now (only the ${stale.length} proved stale — never the whole set).`);
285+
stale.map((s) => `pnpm --filter @objectstack/spec ${s.gen}`).join(' && '));
286+
console.log(
287+
autoFixable.length
288+
? `\nOr re-run with --fix to do it now (only the ${autoFixable.length} proved stale — never the whole set` +
289+
(autoFixable.length < stale.length
290+
? `, and never the ${stale.length - autoFixable.length} ratchet(s) above: read their verdict first).`
291+
: `).`)
292+
: `\n--fix will not do this for you: every stale artifact above is a directional ratchet, ` +
293+
`and its gate already said which direction it moved.`,
294+
);
246295
process.exit(1);
247296
}
248297

249-
console.log(`\n--fix: regenerating the ${stale.length} stale artifact(s). Review the diff before committing.\n`);
298+
console.log(
299+
`\n--fix: regenerating ${autoFixable.length} of the ${stale.length} stale artifact(s)` +
300+
(autoFixable.length < stale.length ? ` — the rest are ratchets, refused below` : '') +
301+
`. Review the diff before committing.\n`,
302+
);
250303
let failed = 0;
251304
for (const s of stale) {
305+
// A ratchet's gate has already answered the question --fix would have to guess:
306+
// it names, per file, whether the debt grew (fix the code) or shrank (re-record
307+
// the number). Regenerating on the first reading launders new debt in as a
308+
// mechanical diff — the same "admit it via the fix command" hazard that keeps
309+
// dual-source-exports.baseline.json out of GATED entirely (#4446). That ledger
310+
// can stay hand-edited because it holds a handful of rows; this one holds 79
311+
// files, so it ships a generator and puts the refusal here instead.
312+
if (s.ratchet) {
313+
failed++;
314+
console.log(` ✗ ${s.gen} — REFUSED`);
315+
console.error(
316+
` ${s.artifact} is a directional debt ledger, not a snapshot of the source.\n`
317+
+ ` "the debt shrank — re-record it" and "the debt grew — fix the new errors" both\n`
318+
+ ` reach --fix as one stale artifact, and only ${s.check} knows which it was.\n`
319+
+ ` Read its per-file verdict; if re-recording is what it asked for, run:\n`
320+
+ ` pnpm --filter @objectstack/spec ${s.gen}`,
321+
);
322+
continue;
323+
}
252324
// The `readsDist` warning above is advice a reader can ignore; here it must
253325
// become a refusal. `gen:api-surface` on a stale dist does not fail — it
254326
// writes a plausible surface with every export added since the last build

0 commit comments

Comments
 (0)