ci: typecheck the test tree too (C-03) - #404
Merged
Merged
Conversation
`tsconfig.json` excludes `src/__tests__`, so the 180-file suite has never been type-checked. Removing that exclude does not work: the root project pins `rootDir: "src"`, and tests import across into `scripts/` (`adoption-wall.test.ts` -> `scripts/update-adoption-wall.ts`), which yields 11 TS6059 rootDir violations. Add `tsconfig.test.json` instead: extends the root project, drops `rootDir` (meaningless under `noEmit`) and `declaration`, and includes `src` with the tests plus the two vitest configs. `pnpm typecheck` now runs it between the root and scripts projects, so the existing `typecheck` CI job covers the suite with no workflow change. Then fix the 115 errors across 29 files it surfaces. No suppressions — no `as any`, no `@ts-ignore`, no widened excludes: - `HandlerDefaults.replaySpeed` is required; test `defaults` literals omitted it. - `createMockRes().writeHead` only modelled one of the two real overloads; it now handles `(status, statusMessage, headers)` as well. - `JournalEntry.body` is nullable; assertions now say so. - `let x: T | undefined = await new Promise(...)` never narrowed — the promises are now explicitly generic. - `AGUIRunAgentInput` requires `threadId`/`runId` and `AGUIMessage` requires `id`; test inputs now supply them, and two `as AGUIRunAgentInput` casts are gone because the literals are now valid. - `isFamilyStillReferenced` takes one argument; four calls passed two. - `recorder-path-traversal` used `endpoint: "/v1/chat/completions"`, which is not in the `FixtureMatch["endpoint"]` union — it is `"chat"`. - `competitive-matrix.test.ts` imported a `.ts` extension; every other test uses `.js`, which resolves the same under NodeNext. - Remaining cast-shaped errors are fixed by reading through the type that already exists (`SSEChunk.system_fingerprint`, the `ResponsesSSEEvent` index signature) or by one documented, single-site widening helper (`looseEntry`, `isRestorableSpy`). Suite unchanged: 5592 passed / 46 skipped, 180 files.
commit: |
jpr5
added a commit
that referenced
this pull request
Sep 8, 2026
…eturns The merge with main reddened `pnpm run typecheck`, which type-checks the TEST tree too (tsconfig.test.json, added in #404 after this branch was cut) — `tsc -p tsconfig.json` alone stays green, which is why this survived the local check that only ran the src config. `readRecordedFixture` read `FixtureFile.fixtures[0]` and declared it a runtime `Fixture`. Those are different types: `FixtureFile.fixtures` is `FixtureFileEntry[]`, the on-disk JSON shape, and `Fixture` is what the server holds after `entryToFixture` converts it. The helper was asserting the conversion had already happened. Return the entry type it actually reads, and convert at the two places that need a runtime fixture — the `createServer` call and the `validateFixtures` assertions — with `entryToFixture`, which is exactly what the real load path does (`handleControlAPI` converts before validating). That also makes the "a fixture the recorder writes must pass load-time validation" assertion stronger than it was: it now runs the same convert-then-validate sequence the loader runs, instead of validating a value mistyped as already-converted. pnpm run typecheck (all three configs) exit 0; full suite 183 files / 5690 tests passing; eslint and prettier clean.
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.
What
tsconfig.jsonexcludessrc/__tests__, so the 180-file / 5,638-test suite has never been type-checked. ThetypecheckCI job added in #402 coverssrc/andscripts/only.Removing the
excludeline does not work. The root project pinsrootDir: "src", and tests import across intoscripts/— that produces 11TS6059rootDir violations. This needed its own project, as the plan said.The tsconfig split
New
tsconfig.test.json:extends: "./tsconfig.json"— samestrict, same target/module/resolution, so the tests are held to the same bar assrc/.rootDir: "."anddeclaration: false— both are emit-shape settings, and this project is--noEmit. DroppingrootDiris what removes the 11TS6059s 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 typechecknow runstsc -p tsconfig.json && tsc -p tsconfig.test.json && tsc -p scripts/tsconfig.json. No workflow change — the existingtypecheckjob already runspnpm 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
3a1b1deBy 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.
TS6059shows as 0 here only because this config dropsrootDir. The naive fix still hits all 11 — see proof 1b below.No suppressions
No
as any, no@ts-ignore, no@ts-expect-error, no neweslint-disable, no widenedexclude, no file relocation.git diff | grep -E '^\+.*(as any|@ts-ignore|@ts-expect-error|eslint-disable)'→ empty.Real defects the gate caught:
isFamilyStillReferencedtakes one argument; four calls inmodels.drift.tspassed two. The second argument was silently discarded.recorder-path-traversal.test.tssetendpoint: "/v1/chat/completions", which is not in theFixtureMatch["endpoint"]union at all. The value is"chat".HandlerDefaults.replaySpeedis required and 24 testdefaultsliterals omitted it.createMockRes().writeHeadmodelled only one of Node's two overloads — it would have thrown away headers on a(status, statusMessage, headers)call.competitive-matrix.test.tsimported a.tsextension; 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). Twoas AGUIRunAgentInputcasts are deleted because the literals became valid.Where a cast was genuinely needed, it is one documented single-site helper (
looseEntryinfix-drift.test.ts,isRestorableSpyinfixture-loader.test.ts) rather than a scatter of double casts.Red-green proof
1 — RED, the real state.
origin/main@3a1b1dein a clean worktree,tsconfig.test.jsondropped in, nothing else changed:1b — the structural blocker is real. On the same pristine tree, the naive fix (delete
src/__tests__fromexclude, keeprootDir):Exactly the 11 the plan predicted.
2 — GREEN on the real tree.
3 — the gate FAILS on a deliberate error. Appended to
src/__tests__/jsonrpc.test.ts:4 — removed, green again.
Gates
prettier --check .npx eslint .pnpm typechecknpx vitest runnpx commitlint --from origin/main --to HEADSuite 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.