fix(metadata): repair the 89 hidden test-type errors and wire a typecheck script - #14627
fix(metadata): repair the 89 hidden test-type errors and wire a typecheck script#14627os-musk wants to merge 4 commits into
typecheck script#14627Conversation
…check` script `packages/metadata/tsconfig.json` declared a program over `src/**/*` — every test file included — while `package.json` declared no `typecheck` script, so `turbo run typecheck` never selected the package and nothing ever ran that program. Run it and it did not compile: 89 errors across 12 test files, all pre-existing, checked by nothing. Route ruled by precedent, not fresh judgement: #13978 took exactly this for the sibling `@objectstack/metadata-protocol` and it landed. Excluding `**/*.test.ts` is refused — it buys an honest config by giving up the coverage, against this repo's declared = enforced direction. Three mechanical families, no assertion moved and no non-test source touched: * 25 TS2835 — relative imports wanting an explicit `.js` extension under node16 resolution, across 12 files. Every rewritten specifier is relative; no package import (`@objectstack/*`, `vitest`, `node:*`) was touched. * 30 TS2345 — one shared mock `PluginContext` literal in metadata.test.ts, missing `registerServiceFactory` and `getServiceScoped`. Closed ONCE, in the single `createMockPluginContext` factory, with inert stubs: neither member is called anywhere in this package. * 20 TS2322 + 1 TS18048 + 1 TS6133 — the residue the config repair EXPOSED. `(evt) => arr.push(evt)` in a watcher slot typed `void | Promise<void>` returns `number`, and the void-return assignability rule does not forgive it because the target is a union; brace-wrapping the concise body is behaviour-identical. Plus one possibly-undefined read and one unused type import. Repairing the config tier first took 89 -> 52: all 33 TS7006 dissolved (they were the cascade off the unresolved imports, not noise this package owed) while 21 errors the 89 had never contained appeared. So the ledger's recorded split (code-tier 30 / config 25 / noise 34) understated the code tier by 21 — the ledger's own "fix the config first, then read the residue" lesson, measured again. `@objectstack/metadata` graduates out of the DEBT ledger by that ledger's own prescribed path, with the measurement recorded in its preamble. `tsc --noEmit -p packages/metadata/tsconfig.json` now reports 0 errors over a program that holds all 43 of the package's test files. Fixes #14342 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
📓 Docs Drift CheckNothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs. What this run could not see
Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
`register-notifies-watchers.test.ts` carried a block explaining why its
`loader-interface` import spells `.js`. Two of its sentences were true when
written and are false at this branch's head, and it is this branch that
falsified them:
* "unlike the three extensionless imports above it" — after the codemod the
relative imports above it carry `.js`, so the contrast has nothing left to
contrast with. (It was already loose: only two relative imports sat above
it, the third being a package specifier the sentence's own reasoning is not
about.)
* "this package's pre-existing type-check debt (#4311) ... left for whoever
pays that ledger down" — this branch IS that payment. The comment told the
next reader to leave alone the thing the same change repaired.
The reason the block exists is kept, because it is exactly what this change
proves: under `moduleResolution: nodenext` an extensionless relative import does
not resolve and every symbol it names silently becomes `any`, which is what
makes `implements MetadataLoader` on the fixture below a check rather than
decoration. What replaces the two false sentences is the fact that now holds —
the package declares a `typecheck` script, so `turbo run typecheck` compiles
this file and a regression here is a red gate rather than a silent `any`.
Also makes the DEBT-preamble graduation note quote the row it replaces
verbatim ("code-tier 30 (TS2345 x30); config-tier 25 (TS2835 x25); noise 34
(TS7006 x33, TS6133)") instead of paraphrasing the split.
Both edits are comments. Re-verified on this head: `pnpm --filter
@objectstack/metadata typecheck` 0 errors, 43 test files / 677 tests green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
|
Not flipped, and the reason is not this PR — engine execution seat, session This PR is ACCEPTed (comment 5512521458 on #14342) and its enqueue bar is otherwise met, but one check on
Established as not this PR's, by measurement rather than by assumption:
⛔ The fix is not ported into this PR. A lockfile bump or a Standing state of this PR: draft, ACCEPTed, not enqueued. It flips the moment the lockfile carries the fix and this check goes green; no change is owed from its author. The other three checks that were still running at 16:1xZ ( One note for whoever lands it afterwards, carried from the review: this branch is ~21 commits behind Generated by Claude Code |
Fixes #14342
packages/metadata/tsconfig.jsondeclared a program oversrc/**/*— every test file included — whilepackage.jsondeclared notypecheckscript, soturbo run typechecknever selected the package and nothing ever ran that program. Run it and it did not compile: 89 errors across 12 test files, all pre-existing, checked by nothing.Route ruled by precedent on the card, not by fresh judgement: PR #13978 took exactly this for the sibling
@objectstack/metadata-protocoland it landed. Option 2 (excluding**/*.test.ts) was refused there and is refused here — it buys an honest config by giving up the coverage, against this repo's declared = enforced direction. The 89 are repaired, never ledgered (test-typecheck-debt.jsonis shrink-only and maintainer-only, per #5286).Head sha:
ef901e559. Every number below was measured onb195676a7, the implementation commit, after the last commit of that round;ef901e559is the patch round at the bottom of this body and touches only two comments, with the readings it re-took listed there.Re-measured composition, and where the card's summary was wrong
Baseline on this branch's base (
7085f9053), dependency closure built first (pnpm --filter '@objectstack/metadata^...' build, exit 0), thentsc --noEmit -p packages/metadata/tsconfig.json:89 exactly, across the same 12 files the card tabulates, per-file counts identical. The card calls these "the two mundane kinds"; there are four codes and the ledger's own note already knew of a third family (TS2345 x30).
The interesting part is what happened after the config tier was repaired. Fixing only the 25 TS2835 took the pile from 89 to 52:
All 33 TS7006 dissolved — they were the cascade off the unresolved imports, not noise this package owed — while 21 errors the 89 had never contained appeared. So the true code tier was 51, not the 30 the ledger recorded. This is the ledger's own standing lesson ("a config-tier count is an upper bound on nothing: fix the config first, then read the residue"), measured a second time;
@objectstack/restis the first, and its TS18048 x13 is the same class of exposure.The three repair families
1. TS2835 x25 — the codemod (12 files). Relative imports wanting an explicit
.jsextension under node16 resolution. Applied from tsc's own diagnostics, and each rewrite was cross-checked against tsc's suggested specifier before being written; the script threw rather than writing on any mismatch. Verified from the diff that every one of the 25 changed lines is a relative specifier — no@objectstack/*, novitest, nonode:*import was touched.2. TS2345 x30 — one shared fixture, fixed once. All 30 are the same mock
PluginContextliteral inmetadata.test.ts(571 through 908), and tsc names the whole defect:is missing the following properties from type 'PluginContext': registerServiceFactory, getServiceScoped. Both are required members ofPluginContextin@objectstack/core; both were added as inertvi.fn()stubs to the singlecreateMockPluginContextfactory, closing all 30 call sites at one site. The stubs are provably inert here — neither member is referenced anywhere in this package's sources. No assertion moved, noas any, no widening of the parameter type.3. TS2322 x20 + TS18048 x1 + TS6133 x1 — the residue the config repair exposed. The 20 TS2322 are one shape twenty times over:
(evt) => arr.push(evt)passed tosubscribe, whose callback return type is the union ofvoidand a void-resolvingPromise(spelled without angle brackets here so the body sanitizer cannot eat the type argument).Array.prototype.pushreturnsnumber, and TypeScript's void-return assignability rule does not forgive that here because the target is that union rather than barevoid. Brace-wrapping the concise body ((evt) => { arr.push(evt); }) is behaviour-identical —subscribediscards the return value either way — and it is the smallest edit that stops the value escaping into the slot. Plus one possibly-undefined read (diffResult.patch!.length, a non-null assertion, which is this package's existing convention in tests and which no rule ineslint.config.mjsforbids) and one unused type import removed.⛔ No non-test source file under
src/was touched, in any family.The deliverable's proof
The script name is echoed, so this is not a zero-match
--filterrun exiting 0 having run nothing.The program really holds the tests.
tsc --noEmit --listFilesover the package: 43 of 43*.test.tsfiles on disk are in the program, none missing — includingsrc/loaders/filesystem-loader-keyed-items.test.ts, the pin file that landed after the card was written, which is clean. Atypecheckthat excludes the files it advertises is the exact failure mode this gate family exists for, so it is measured rather than assumed.The gate now bites (reverse verification, not an ablation). Reverting exactly one
.jsextension this PR added:The mutation was proved on disk before the run (anchor counts both directions, plus a blob hash differing from HEAD), and the restore is proved by reproducing the HEAD blob hash exactly, not by an exit code. No rebuild leg is owed or claimed: tsc reads this package's own
src/directly, so nodist/sits between the mutation and the measurement — only the dependency closure is dist-resolved, and it was neither mutated nor restored.Turbo selection
turbo run typecheck --filter @objectstack/metadata --dry-run=jsonnow emits the task:And the root
pnpm typecheck(turbo run typecheck) has 79#typechecktasks,@objectstack/metadata#typecheckamong them. Both readings taken onb195676a7.Ledger update, by the gate's own path
check:type-check-coveragenames the remedy itself, and it was followed rather than invented. With the script added but the row still present:The DEBT preamble prescribes the same thing in prose ("fix (or config-fix) its errors, add a
typecheckscript, and delete its entry here in the same PR"). So the row was deleted, not hand-lowered to 0 —--lowerwrites a measured number back for an entry that still stands, and this one graduated. A graduation note recording the 89 to 52 measurement was added to the DEBT preamble, matching the convention therest/plugin-security/trigger-record-changenotes established.After:
Was 69/79 covered, 10 entries, 305 frozen raw errors. ⛔ No
test-typecheck-debt.jsonentry was added: this package's tsconfig hides no tests, so it was never in TEST_DEBT's hole, and that ledger is shrink-only and maintainer-only anyway.skip-changeset— measured, not assumedThe label is applied, on the rule the workflow states for itself ("this PR declares no release of its own"). Measurement rather than reasoning:
packages/metadata/tsup.config.tsdeclares four entries (src/index.ts,src/node.ts,src/migrations/index.ts,src/errors.ts) — no test file among them.dist/, 95 source inputs in total, zero are*.test.ts. Grep of every shipped.js/.cjs/.d.ts/.d.ctsforcreateMockPluginContext,vitestanddescribe(: 0 hits. (The.test.strings that do appear indistare prose inside docblocks ofsrc/metadata-manager.ts, a file this PR does not touch, carried throughsourcesContent.)packages/metadata/srcis a*.test.tsand therefore an input to nothing published. The only other change inside the package is ascripts.typecheckentry inpackage.json— dev tooling metadata, no runtime behaviour, the identical change PR [finding]@objectstack/metadata-protocolhas notypecheckscript: its only type coverage is the tsup dts build, which reports one file per run and never reaches the 138 test files #13978 made for the sibling package.So
dist/is invariant under this diff and no user-visible behaviour moves.Clause-② self-reading: no
Read off this diff, not off the dispatch note. 12 test files (mechanical typing repairs, no assertion moved, no accept/reject behaviour touched), one
scriptsentry in apackage.json, and one gate ledger row deleted with its graduation recorded. No public type changes, no parse or validation behaviour changes, and no gate is weakened — a gate is ADDED (this package'stypechecknow runs underturbo run typecheck) and the DEBT ledger shrinks by one entry and 89 frozen errors.Verification
Union re-derived on the final tree with
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands(no path list) after the last commit — 14 paths vs merge base7085f9053, 44 gates — and run. Exit codes captured after a redirect, never through a pipe.Green (exit 0):
pnpm lint(whole repo,eslint . --no-inline-config, exit 0 — not narrowed) ·pnpm check:nul-bytes(plus a control-byte self-scan over the diff, 0 hits) ·pnpm check:error-status-conformance·pnpm --filter @objectstack/metadata typecheck·pnpm --filter @objectstack/metadata exec vitest run(43 test files, 677 tests, all passed — assertions untouched) ·pnpm --filter @objectstack/metadata build(8/8 declaration files present) ·check:type-check-coverage·check:type-source-resolution·check:turbo-task-graph·check:published-files·check:pnpm-filter-targets·check:workspace-manifest-cycles·check:test-source-alias·check:ratchet-remedy-authority·check:service-providers·check:query-options-erasure·check:slot-lookup·check:watch-hint-literal·check:where-matcher·check:engine-double-contract·check:entry-guard·check:parse-guard·check:doc-authoring·check:override-consistency·check:page-declaration-shape·check:durability-log-level·check:logger-receiver-detach·check:objectql-double-limit·check:cli-command-ids·check:agent-test-spelling·check:bash32-floor·check:cross-package-test-inputs·check:llms-txt·check-changeset-fixed·check-ci-filter-parity·check-comment-mask-adoption·check-cross-package-test-inputs·check-keyed-text-bounds·check-osv-exemptions·check-plugin-teardown-shape·check-shard-attestation·check-system-context-census·check-undeclared-dep-imports·docs-audit/check-affected-docs·docs-audit/check-drift-comment·pm/bare-root-worklist --self-test.Edited gate script — its own suite. This PR edits
scripts/check-type-check-coverage.mjs, so its own tests are owed on top of the derived family. Repo-wide grep finds no vitest file that executes it (all 12*.test.tsmatches are prose citations in comments), so its suite is its built-in--self-test, which bothcheck:type-check-coverageandcheck:type-check-debtchain and which passed: 48 semantic + 68 observation + 43 re-measure + 28 built-closure + 19 auto-lowering + 18 exit-code cases.NOT MEASURED locally — declared, with the reason (four gates). None is a red, and each is CI-owned:
check:type-check-debt— exit 3, the gate's own PREREQUISITE-NOT-MET code, in its own words "NOT a pass and NOT a finding: nothing was measured". It names 42 unbuilt workspace dependencies and the remedy (turbo run build --filter='./packages/*' --filter='./packages/*/*'— a whole-workspace build; 60 packages here have nodist), which is a steplint.ymlruns immediately before it and which does not fit this container's ~10-minute foreground cap from cold. The substitute measurement for the entry this PR removes is the one that gate would have made:tsc --noEmitover the package with its dependency closure built, which is the run above at 0 errors. No other ledger entry's package is touched by this diff.check:test-completeness— exit 3, same class: it grades a savedturbo run testlog and none was supplied. Its own message prescribes the disposition — "running the family locally, record this gate as NOT MEASURED".check:dual-build-cjs-loads— exit 3: it reads built output and 52 packages have nodistin a fresh worktree. "Runpnpm buildfirst. This is NOT a pass: nothing was measured."check:pm-dispatch-gates— killed by the container's foreground cap on three attempts (540s, 540s, 575s), each getting further, with 0 failing assertions in all three. It is a thin wrapper around one longdispatch-gates.mjs --self-test. The assertion in it that actually reads the file this PR edits — "the type-check ledger gate declares the root-program script whose errors it ratchets" — ran and passed before the kill.CI runs the whole farm exactly once on a built tree, which is where these four land.
Out of scope, filed not fixed
packages/metadata/src/metadata-history.test.tssits behind anif (manager.X)guard over a method that is unconditionally declared — 8 sites that go green asserting nothing #14623 — every assertion inpackages/metadata/src/metadata-history.test.tssits behind anif (manager.X)guard (8 sites) over methods that are unconditionally declared onMetadataManager, so the file goes green asserting nothing the moment one of them moves. Noticed while repairing the TS18048 on line 150 of that file; repairing it is an assertion move, which this card's ruling excludes.Related family, named so it is not re-filed: PR #13978 (first member,
@objectstack/metadata-protocol) and the newly filed #14613 (@objectstack/core, same shape, 98 errors). ⛔ Neither is addressed here and this PR does not widen to cover them. The general shape is worth a census question for triage: a package whose tsconfig declares a program oversrc/**/*but whosepackage.jsondeclares notypecheckscript is never selected byturbo run typecheck, so the config reads as coverage and delivers none — three instances are now known, andcheck:type-check-coverage's COVERED invariant catches only the packages that carry a ledger row, not the ones whose absence of a row was never noticed.Patch round —
ef901e559Review found that the codemod falsified a comment in one of its own files, and it did.
packages/metadata/src/register-notifies-watchers.test.tscarried a block explaining why itsloader-interfaceimport spells.js, and two of its sentences were true when written and false at the head of the round before this one:.js, so the contrast had nothing left to contrast with. (It was already loose: only two relative imports sat above it, the third being a package specifier the sentence's own reasoning is not about.)The block is kept, because the reason in it is exactly what this branch proves: under
moduleResolution: nodenextan extensionless relative import does not resolve and every symbol it names silently becomesany, which is what makesimplements MetadataLoaderon the fixture below a check rather than decoration. What replaces the two false sentences is the fact that now holds — the package declares atypecheckscript, soturbo run typecheckcompiles this file and a regression here is a red gate rather than a silentany.Swept the rest of the diff for the same shape, and the result is none. Two greps over the changed set (
git diff --name-only MERGE_BASE HEAD, 14 paths):Across the 12 touched test files the only hits are the rewritten block itself and one unrelated line in
database-loader.test.ts("rowToRecordreachescreatedAt/updatedAtthrough an unchecked cast ... which is why tsc reported nothing") — that sentence is about anascast defeating tsc on one field, it is untouched by this diff, and a cast still defeats tsc after the script lands, so it is not falsified. Inscripts/check-type-check-coverage.mjsevery hit is either the generic top-of-ledger NodeNext lesson or another package's entry (rest,cli), plus three sha-anchored past-tense worked examples that cite this package's old numbers as history ("were repaired by hand", "on the trees that carried them: at 699132f", "the fixtures are the real instances, transplanted") — none claims a live row. The decisive check for that: after the deletion the ONLY surviving occurrence of@objectstack/metadatain that file is the graduation note itself.Graduation note quotation made exact. It paraphrased the deleted row's split; it now quotes the row's opening itemisation verbatim —
code-tier 30 (TS2345 x30); config-tier 25 (TS2835 x25); noise 34 (TS7006 x33, TS6133)— which also makes the "ALL 33 TS7006 dissolved" line beside it land against the exact figure it overturns.Both edits are comments. Re-verified on
ef901e559: dependency closure rebuilt,pnpm --filter @objectstack/metadata typecheck0 errors,vitest run43 files / 677 tests passed,pnpm lint(whole repo) exit 0,pnpm check:nul-bytesexit 0 plus a control-byte self-scan over the two changed files with 0 hits, and the comment-sensitive and ratchet gates re-run green:check:type-check-coverage(still "OK — 70/79 ... 9 in the DEBT ledger (216 frozen raw errors)"),check-comment-mask-adoption,check:entry-guard,check:parse-guard,check-keyed-text-bounds,check:ratchet-remedy-authority,check:test-source-alias,check:agent-test-spelling. The gate family re-derived onef901e559is byte-identical to the one derived onb195676a7, so nothing new was owed.One reading worth passing on rather than acting on:
dispatch-gates.mjsnow prints a STALE TREE warning on this branch — at least 21 commits behindorigin/main, with 9 files the derivation reads changed across that range. The family is unchanged, and CI derives against the merge result, so this is a note for the landing seat, not a finding.🤖 Generated with Claude Code
https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68