ci: type-check the repo, and make type-checking safe to run - #402
Merged
Merged
Conversation
Neither tsconfig set noEmit, so running tsc -p directly emitted output into
tracked and published directories:
tsc -p scripts/tsconfig.json -> 12 .js files next to their .ts sources in
scripts/, plus 8 more in src/ (reached via
the scripts' relative imports)
tsc -p tsconfig.json -> 47 .d.ts/.d.ts.map/.js.map files into dist/,
the directory tsdown owns and the package
publishes
Both are latent only because nothing runs tsc today; adding a typecheck gate
arms them. tsdown is unaffected: pnpm build still exits 0 with an identical
555-file dist tree and all 68 .d.ts/.d.cts pairs, and pnpm test:exports stays
green.
Nothing in this repo ever ran tsc: no typecheck script, and none of the four Static Quality jobs invoked it. The scripts/ tree runs unattended in CI via tsx, which strips types without checking them, so a type error there first surfaces as a runtime failure of the weekly job. Adds a typecheck script covering both existing projects (tsconfig.json -> src/, scripts/tsconfig.json -> scripts/), a matching CI job, and typecheck to the release chain. Both projects already pass on this tree, so the gate is green from the start; it is scoped to what passes today and deliberately does not cover src/__tests__, which tsconfig.json excludes and which currently has 126 errors (a separate change requiring a rootDir redesign). Note the repo has no required status checks, so this alerts rather than blocks.
commit: |
jpr5
added a commit
that referenced
this pull request
Sep 2, 2026
## What `tsconfig.json` excludes `src/__tests__`, so the 180-file / 5,638-test suite has **never** been type-checked. The `typecheck` CI job added in #402 covers `src/` and `scripts/` only. Removing the `exclude` line does not work. The root project pins `rootDir: "src"`, and tests import across into `scripts/` — that produces **11 `TS6059`** rootDir violations. This needed its own project, as the plan said. ## The tsconfig split New `tsconfig.test.json`: - `extends: "./tsconfig.json"` — same `strict`, same target/module/resolution, so the tests are held to the same bar as `src/`. - `rootDir: "."` and `declaration: false` — both are emit-shape settings, and this project is `--noEmit`. Dropping `rootDir` is what removes the 11 `TS6059`s without weakening a single check. - `include: ["src", "vitest.config.ts", "vitest.config.drift.ts"]`, `exclude: ["node_modules", "dist"]` — the tests are in, and the two vitest configs get covered for free. `pnpm typecheck` now runs `tsc -p tsconfig.json && tsc -p tsconfig.test.json && tsc -p scripts/tsconfig.json`. **No workflow change** — the existing `typecheck` job already runs `pnpm typecheck`. Program membership, not just a green exit: `tsc -p tsconfig.test.json --listFiles | grep -c src/__tests__` = **223** (vs **0** for the root project). ## Measured, not carried forward | | plan | measured on `3a1b1de` | |---|---|---| | errors | 126 | **115** | | files | ~40 | **29** | By code: 38×TS2345, 21×TS2352, 17×TS2531, 8×TS18048, 6×TS2339, 6×TS2322, 5×TS2769, 4×TS2683, 4×TS2554, 3×TS2741, 1×TS5097, 1×TS2353, 1×TS18047. `TS6059` shows as 0 here only because this config drops `rootDir`. The naive fix still hits all 11 — see proof 1b below. ## No suppressions No `as any`, no `@ts-ignore`, no `@ts-expect-error`, no new `eslint-disable`, no widened `exclude`, no file relocation. `git diff | grep -E '^\+.*(as any|@ts-ignore|@ts-expect-error|eslint-disable)'` → empty. Real defects the gate caught: - **`isFamilyStillReferenced` takes one argument; four calls in `models.drift.ts` passed two.** The second argument was silently discarded. - **`recorder-path-traversal.test.ts` set `endpoint: "/v1/chat/completions"`**, which is not in the `FixtureMatch["endpoint"]` union at all. The value is `"chat"`. - **`HandlerDefaults.replaySpeed` is required** and 24 test `defaults` literals omitted it. - **`createMockRes().writeHead` modelled only one of Node's two overloads** — it would have thrown away headers on a `(status, statusMessage, headers)` call. - **`competitive-matrix.test.ts` imported a `.ts` extension**; every other test uses `.js`, which resolves the same under NodeNext. The rest are honest assertions where the type is genuinely nullable (`JournalEntry.body`), narrowing that never happened (`let x: T | undefined = await new Promise(...)` — the promises are now explicitly generic), or missing required fields (`AGUIMessage.id`, `AGUIRunAgentInput.threadId`/`runId`, `SSEChoice.logprobs`). Two `as AGUIRunAgentInput` casts are **deleted** because the literals became valid. Where a cast was genuinely needed, it is one documented single-site helper (`looseEntry` in `fix-drift.test.ts`, `isRestorableSpy` in `fixture-loader.test.ts`) rather than a scatter of double casts. ## Red-green proof **1 — RED, the real state.** `origin/main` @ `3a1b1de` in a clean worktree, `tsconfig.test.json` dropped in, nothing else changed: ``` RED RC=2 115 errors 29 files ``` **1b — the structural blocker is real.** On the same pristine tree, the naive fix (delete `src/__tests__` from `exclude`, keep `rootDir`): ``` $ npx tsc -p tsconfig.naive.json --noEmit 2>&1 | grep -c "error TS6059" 11 scripts/drift-report-collector.ts(41,8): error TS6059: File '.../scripts/drift-types.ts' is not under 'rootDir' '.../src'. ``` Exactly the 11 the plan predicted. **2 — GREEN on the real tree.** ``` $ pnpm typecheck ; echo $? > tsc -p tsconfig.json --noEmit && tsc -p tsconfig.test.json --noEmit && tsc -p scripts/tsconfig.json --noEmit 0 ``` **3 — the gate FAILS on a deliberate error.** Appended to `src/__tests__/jsonrpc.test.ts`: ```ts const deliberateTypeError: number = "C-03 gate must catch this"; ``` ``` $ pnpm typecheck ; echo $? src/__tests__/jsonrpc.test.ts(517,7): error TS2322: Type 'string' is not assignable to type 'number'. ELIFECYCLE Command failed with exit code 2. 2 ``` **4 — removed, green again.** ``` $ pnpm typecheck ; echo $? 0 ``` ## Gates | gate | exit | |---|---| | `prettier --check .` | 0 | | `npx eslint .` | 0 | | `pnpm typecheck` | 0 | | `npx vitest run` | 0 | | `npx commitlint --from origin/main --to HEAD` | 0 | **Suite unchanged against baseline:** 179 passed / 1 skipped (180 files), 5592 passed / 46 skipped (5638 tests). No workflow touched, so no `actionlint`. No dependency change, so no lockfile drift. ## Nothing left unfixed All 115 are fixed. Zero remainder.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two changes, ordered: make
tscsafe to run, then make it run.noEmiton both tsconfigsNeither tsconfig set
noEmit, so invokingtsc -pwrote build output into tracked and published directories. Observed on the pre-fix tree:After adding
"noEmit": trueto both, the same two commands emit nothing:scripts/*.jscount 0,dist/not created.Non-regression —
noEmitdoes not disturb tsdown:(The dist tree hash is nondeterministic build-to-build, so the file set is the signal here, not a hash.)
typecheckscript + CI jobgrep "typecheck\|tsc "overpackage.jsonand every workflow returned nothing — no type-check gate existed anywhere. The configs were already fine; only the runner was missing. Addspnpm typecheck(both projects,--noEmiton the CLI as well as in the configs), atypecheckjob in Static Quality matching the existing jobs' pinned SHAs and pnpm setup, andtypecheckto thereleasechain.A new gate is only worth something if it has been watched fail. Proven locally with throwaway probe files, one per project, neither committed:
What the gate covers, and what it does not
src/(viatsconfig.json) andscripts/(viascripts/tsconfig.json) — including the 83 KB of unattended CI TypeScript that runs throughtsx, which strips types without checking them.src/__tests__, whichtsconfig.jsonalready excludes. Including it produces 126 errors across ~40 files (38×TS2345, 21×TS2352, 17×TS2531, 11×TS6059rootDirviolations, …) and needs its own tsconfig. That is separate work, deliberately not attempted here and not papered over withexcludehacks — the exclusion is the pre-existing config, unchanged.Gates